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

Traits (Category) should differentiate test cases even when they have same data name #5097

Closed
PeterHevesi opened this issue Jun 12, 2024 · 11 comments

Comments

@PeterHevesi
Copy link

PeterHevesi commented Jun 12, 2024

Hi there,
I have found, that only because same dataset has the very same name as another dataset coming from different datasource, it is not discovered in VS Test Explorer. But it should, because by putting different Category (from NUnit), I already want to clearly say, that that datasource is actually different.
Even when I use Traits to group my tests, these tests are never discovered when having the same name.
I have made simple NUnit Test project with this simple file, which should absolutely clearly illustrate what I mean:

https://gist.github.com/PeterHevesi/aa1392c27000985c13c35dff9653f83a

@nohwnd
Copy link
Member

nohwnd commented Jun 12, 2024

Looking at your code I don't see anything vstest specific, this looks like a problem specific to NUnit. Have you tried filing the issue in Nunit? https://github.com/nunit/nunit

@PeterHevesi
Copy link
Author

Well I have created an issue in nunit3-vs-adapter, here:
nunit/nunit3-vs-adapter#1182

And they told me, that it is not on their side, but it is how Test Explorer is identifying tests by FQN. But couldn't category be put into that FQN? So just by having different category (traits) on a testcasesource, it could have the very same name and would be properly found as different test cases?

Don't know why Category is not put into that FQN to differentiate tests, it doesn't seems right to me, and it makes working with testCaseSources harder!

@nohwnd
Copy link
Member

nohwnd commented Jun 18, 2024

This would be a big change, but you can file an issue to TestExplorer by pressing the feedback button in VS. It would also require nunit to add that information to the fully qualified name so it becames distinct.

@PeterHevesi
Copy link
Author

PeterHevesi commented Jun 18, 2024

Ok thanks, I have filed an issue there :)
I hope they will fix this. It is only very logical to work like I am proposing :)
https://developercommunity.visualstudio.com/t/Traits-Category-should-differentiate-t/10685328?port=1025&fsid=ff50b090-f972-4576-800e-7eb71583d2e2

@PeterHevesi
Copy link
Author

This work could also try to solve another problem, that is currently there. We are discussing it with @OsirisTerje at this issue:
nunit/nunit3-vs-adapter#1182 (comment)

As I said, it could only be very logical to even add another attributes besides category, as follows:

  1. CategoryGroup
  2. Category
  3. Subcategory
  4. Tag

or even more attributes, if someone would need them.

All of these would be part of FQN, to differentiate tests from different datasources, even when they have same dataname.
Each attribute could be then used separately to categorize by some grouping, for example in my scenario, we have different website version for each of our customer, I would use Category for grouping by website version.
Then I would also like to group by let's say browser those tests should be run on. I would use Subcategory for that.

And you could of course group by all of these 4 attributes independently in Test Explorer !!!

@PeterHevesi
Copy link
Author

PeterHevesi commented Jun 26, 2024

Hi @nohwnd, so what do you think about this?
Is there any chance to make category to be part of FQN?
It won't change the current behavior in any way, as I understand it.
It would only fix the problem I described at the beginning

PS: If you agree with me, that this change won't do any harm to existing behavior, and everything would work exactly the same after this change, would you please upvote this feature request?
https://developercommunity.visualstudio.com/t/Traits-Category-should-differentiate-t/10685328?

@nohwnd
Copy link
Member

nohwnd commented Jul 2, 2024

I doubt it, every time we touch the fqn or the code that generates IDs we need to make sure that all consumers know about the change and accomodate it.

@PeterHevesi
Copy link
Author

PeterHevesi commented Jul 2, 2024

But what behavior would be changed? Other than repairing a "bug" I was talking about?
I can't get, what would break for anybody...

Just to be sure, do you understand, why I am talking about it as a bug? I am not so sure, if you really understand what I am talking about. For me, there is absolutely no reason why I should write to the name of the test, the same thing I wrote to Category, just to make the TestDataSource to appear in Test Explorer.

Once again, this is not correct at all, and I am really surprised, that it is still working like this.
You put Category into FQN => All tests that are now discoverable will be still discoverable with the very same structure. Don't you agree with me on this? If no, would you please explain to me?

Did you carefully looked at my example at the very beginning? Because I doubt it. If you look at it carefully, you will understand, that this is an issue with an EASY FIX, that doesn't break anything...

THERE IS NOBODY, THAT NEEDS TO ACCOMODATE TO THIS CHANGE !!!
If yes, could you please explain, who will have a need to accomodate to something, that doesn't change anything for them?

PS: Prosím ťa, naozaj sa nad tým trochu zamysli, či by sa to naozaj nedalo úplne v kľude spraviť, pre mňa je to veľmi dôležité.
Som fakt smutný, že nikto ma v tomto vôbec nechápe....
Ďakujem pekne, ak sa na to ešte pozrieš :)

@nohwnd
Copy link
Member

nohwnd commented Jul 2, 2024

If we add category to the FQN that is parsed by VS Test Explorer, then they will need to change their regex that parses it and accommodate the difference. If we do that we should also add it to the ID generator, which would change the IDs generated for the test cases, and that would break TestCase functionality in Azure Devops, the ID for test cases that have category would now be different than before without any user change.

There is a bunch of metadata available already to test explorer (like traits), but category is not part of the hierarchy that we have, that is historically a container, namespace, class, method, data. And we cannot easily change this.

@PeterHevesi
Copy link
Author

PeterHevesi commented Jul 3, 2024

"There is a bunch of metadata available already to test explorer (like traits), but category is not part of the hierarchy that we have"
I meant Traits, it is called Category in NUnit and is mapped to Traits, so you have that :)

Alright, now I understand you well. But still have few more questions:

  1. What if you add some setting to runsettings or somewhere, that would make Traits to be part of FQN? Default behavior would still be the same as it is now, but people would be able to make it work like this?

  2. What about creating TraitGroup and SubTraits alongside Traits?
    I mean right now, the maximum length of grouping is like 4, if you don't need to use (State, Duration, Target Framework or Environment), and it is like this, in any order of course: Project > Namespace > Class > Traits.
    These 4 attributes are the only one that I can customize myself...
    And then, it is only very logical for me, that all tests for our website are in 1 project, because it is much easier to manage it.
    So basically, the length is 3 for me.
    But I would need more groupings indeed. This 2 new attributes would increase the maximum length of grouping by 2 :)
    Would this be possible? By adding new attributes, it wouldn't change anything for Traits attribute, right? If someone is still using only Traits, he would not need to change any parsing or stuff like that, right?

So I still have this vision, of having TraitGroups, Traits and Subtraits and then all 3 of these could be configurable from .runsettings to be part of FQN, if someone need it, like me :D
And it would be possible to group by all 3 of these in Test Explorer.

So what do you think? I don't want to discuss this forever and take your precious time, but this was like my very last question, if you say NO to both of these questions, I will give up :D

@nohwnd
Copy link
Member

nohwnd commented Jul 9, 2024

As said here and in nunit repo we won't be changing the fqn to include category name.

@nohwnd nohwnd closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants