Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/fix categories parsing #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AFAde
Copy link

@AFAde AFAde commented Mar 18, 2019

No description provided.

@anshooarora
Copy link
Member

@AFAde

Which NUnit version contains this element?

@dsparkplug
Copy link
Contributor

Categories are output as property name-value pairs as of NUnit 3. e.g. <property name="Category" value="SuperFancyCategoryName" />

See: https://nunit.org/files/testresult_30.txt

Probably should change this add support for the version 3 XML schema rather than overwriting support for version 2

@AFAde
Copy link
Author

AFAde commented Mar 25, 2019

Categories are output as property name-value pairs as of NUnit 3. e.g. <property name="Category" value="SuperFancyCategoryName" />

See: https://nunit.org/files/testresult_30.txt

Probably should change this add support for the version 3 XML schema rather than overwriting support for version 2

Yes, that's right, that's how categories are handled in the version 3. Having a new dedicated parser for version 3 is definitely a good idea.

@dsparkplug
Copy link
Contributor

Having a new dedicated parser for version 3 is definitely a good idea.

Maybe. But for now, I would suggest editing the pull request to include both the if (parser(elem, "categories").Any()) and if (parser(elem, "properties").Any()) sections of the code so that this parser works with results from all versions of NUnit.

@AFAde
Copy link
Author

AFAde commented Mar 25, 2019

Having a new dedicated parser for version 3 is definitely a good idea.

Maybe. But for now, I would suggest editing the pull request to include both the if (parser(elem, "categories").Any()) and if (parser(elem, "properties").Any()) sections of the code so that this parser works with results from all versions of NUnit.

Speaking in terms of clean code, I am not sure whether that's a good solution; it will fix the issue but it will introduce a kind of code smell, if you ask me. If you want to keep it simple, I would rather give up the compatibility with NUnit 2, which is no longer maintained. But I leave the final decision to you.

@esrahofstede
Copy link

Are there any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants