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

dotnet test: Filter with huge list of FullyQualifiedNames takes long to resolve tests #1084

Open
kales5 opened this issue Mar 29, 2023 · 12 comments
Labels
External Tracked here, but must be fixed in external tool

Comments

@kales5
Copy link

kales5 commented Mar 29, 2023

NUnit: 3.13.3
NUnit3TestAdapter: 4.4.2
.Net: 4.8

We wanted to use the new TeamCity Feature to run test in parrallel on several agents (https://www.jetbrains.com/help/teamcity/cloud/parallel-tests.html).
But the test run time got way up. While investigating this we saw that team city creates .runsetting files in which they put a TestCaseFilter that uses a concantion of FullyQualifiedName~.
Our solution has 30k test in 4300 classes, so the list of this is pretty long, if we paralize the build with 5 agents, the list in each runsetting file is roughly 860 entries of FullyQualifiedName~
When we execute these runsettings against our biggest test projects which has around 22k of tests, it takes 1 hour to start with the tests.
When we just execute dotnet test project.dll the test execution starts after a few seconds.

I attached an example runsetting file so you can understand how it looks in general.

Can you explain this behavior and do you think you could speed this up?

example.runsettings.txt

@OsirisTerje
Copy link
Member

OsirisTerje commented Mar 29, 2023

That sounds like a very bad design, to be honest. Sending such large lists to the engine/framework will take time, and also processing these filters. There was some timing analysis done a few years back, I can try to refind them, but the essence was that the way the operators are specified also matters wrt processing time.
The reason for the time it takes before it starts testing is the pre-test discovery, that is the part that resolves the filter and returns with a list of tests to run to the actual execution. When you run dotnet test project.dll there is no actual filter at all, so it all just runs.
If you enable the DumpXmlTestResults you can see in the file the actual NUnit filter that it has been resolved into.
You should also check to see if the AssemblySelectLimit has been triggered, but it actually doesn't sound like that.

The list of tests being sent down to the engine and framework causes delays, but exactly where the major hit is, we don't know. There are better ways to handle this, e.g. it would be much wiser to split the set of tests up using categories.

Also, if you have split those 30K tests in multiple assemblies, the Azure Pipeline split the tests up automatically per assembly. Now, you're using teamcity, but it can then be easier to do that manually (e.g. with multiple test steps) instead of using a broken (imho) new feature.

You should also reach out to Jetbrains about this and ask them to reconsider their design.

Another issue affected by this Teamcity feature: #1078

@OsirisTerje
Copy link
Member

@kales5 Can you link to any jetbrains issue on this?

@kales5
Copy link
Author

kales5 commented May 5, 2023

Sorry, we abandoned this approach and so I did not dig into it any further.
We have already your suggested approach with the different Test Categories, the problem there is only that one needs to go through the test once in while to evenly distribute the tests between build agents.

I did not create an issue at jetbrains yet, if you want you can close this issue, although I think the test resolve should be quicker even if there are a few thousand test classes in the filter.

@OsirisTerje
Copy link
Member

Thanks! We do several performance enhancements in the framework for V4, so hopefully that will be helping the situation. We will try to have an alpha version out real soon now, so you can test if it helps in your situation.

@mchechulnikov
Copy link

mchechulnikov commented May 5, 2023

Hello, I am from the TeamCity team.

We apologize for the inconvenience you have experienced and want to assure you that we are aware of the problem.

When integrating with testing platforms – Gradle/Maven (JUnit, TestNG), VSTest (NUnit, xUnit, MSTest), etc., – our approach is to be framework-agnostic and attempt to use the native APIs of these tools as much as possible. This is why we strive to use native filtering in all cases.

The automatic test parallelization feature in TeamCity distributes test classes across agents based on each test's runtime statistics. The goal is to distribute tests evenly among agents, ideally leading to a significant speedup. However, when dealing with a large number of test classes in a project, enormous filters are generated. Platforms like Gradle and Maven handle such filters adequately, but we have encountered difficulties with .NET. The situation is better with xUnit, though it is not ideal.

We have tried various optimization strategies, such as minimizing filters by collapsing matching namespaces or building only exact (= and !=) filters. Unfortunately, these efforts have not resulted in substantial performance improvements on average.

Currently, we are experimenting with approaches that involve discontinuing the use of VSTest filters, for example https://youtrack.jetbrains.com/issue/TW-80527

Unfortunately, these experimental approaches have their own drawbacks, which we are currently working to address. Nevertheless, in the main cases we see a significant increase in performance.

If you don't mind, once the teamcity-dotnet-plugin with this experimental solution becomes available, I will write back to this ticket so that you can try it for your specific case. Or please, report us the issue in YouTrack.

@OsirisTerje
Copy link
Member

Thanks @mchechulnikov !

Appreciate you writing back here. I'll reopen this ticket and label it accordingly.

  1. The 80257 sounds interesting. I assume you're aware of having to do that on all the test methods, since the TestFixture attribute is now redundant.

  2. List based filters of any kind are slow. But category-based filters are much faster. So, you could consider another rewrite strategy during build, and just add suitable custom categories to the appropriate tests and use those. That would of course take some time to add those, but might be faster in the end. It could be worth a try. It is another variant of your 80257 approach, which also could work, but it would affect fewer tests as you only need to change those to be executed.

  3. 80257 more: You can add the Explicit attribute to all the test classes that you don't want to run. That would be much fewer than touching all the test cases. They would still be discovered but will be ignored for running.

  4. The adapter and the engine both have the NUnit.Where clause that can be used as an alternate filter. But, it resolves down to the same underlying filters anyway, so it might be some performance to get there, but not as much as you need I think.

@OsirisTerje OsirisTerje reopened this May 5, 2023
@OsirisTerje OsirisTerje added the External Tracked here, but must be fixed in external tool label May 5, 2023
@mchechulnikov
Copy link

mchechulnikov commented May 5, 2023

Thank you @OsirisTerje !

I assume you're aware of having to do that on all the test methods, since the TestFixture attribute is now redundant.

Sure, we perform according to different strategies for various test frameworks and test descriptions. We're aware that sometimes class-level attributes can be redundant.

So, you could consider another rewrite strategy during build, and just add suitable custom categories to the appropriate tests and use those. That would of course take some time to add those, but might be faster in the end. It could be worth a try.

Thank you for the proposal. About changing the distribution mechanism – it sounds doable, but I'm not sure if I understand you correctly – please correct me if I'm wrong.

  • If you're suggesting adding categories dynamically on the TeamCity side, it means that we still need to disassemble an assembly and iterate over all methods and classes in any way to mark all the tests or test classes by categories, right?

  • If you're suggesting distributing according to user-defined test categories, then I'd say that the key idea behind the Parallel tests feature is that TeamCity can parallelize tests across multiple agents without any direct code changes from the user's side. The user may not have all tests/classes marked by categories.

Another concern is that I'm not sure whether the term "test category" is applicable for all test types across all test engines on all the platforms supported by TeamCity.

You can add the Explicit attribute to all the test classes that you don't want to run. That would be much fewer than touching all the test cases. They would still be discovered but will be ignored for running.

You know, right now it seems like it's not an issue to iterate over even hundreds of thousands of methods/classes using disassembled Mono.Cecil assembly. Nonetheless, I still have some doubts about whether it's a good idea – to iterate over all test cases/classes. I haven't considered the suggested strategy for excluding test classes in NUnit. I'll give it a try, thank you again!

The adapter and the engine both have the NUnit.Where clause that can be used as an alternate filter. But, it resolves down to the same underlying filters anyway, so it might be some performance to get there, but not as much as you need I think.

We are aware of this possibility. Before, we tried an approach that didn't require knowledge about test engines under the dotnet test or VSTest, so it wasn't a valid option for us. Right now, the approach excludes any filter statements at all, so I'm not sure how it could be useful for us. Anyway, thank you for the suggestion.

@OsirisTerje
Copy link
Member

If you're suggesting adding categories dynamically on the TeamCity side, it means that we still need to disassemble an assembly and iterate over all methods and classes in any way to mark all the tests or test classes by categories, right?

Yes, this is what I meant. And yes you need to disassemble it, but you only need to add your category to the classes. That should shave off some time.

About supporting different frameworks, you might have to add attributes for categories for all of them, but they are not that many.

@OsirisTerje
Copy link
Member

@mchechulnikov How is this progressing? Any news ?

@mchechulnikov
Copy link

@OsirisTerje we have finished the very first implementation. However, it targets the TeamCity 2023.09 Cloud version for now. Since the .NET plugin for TeamCity is bundled and requires sync changes in TeamCity core, unfortunately, it could be incompatible with the current TeamCity 2023.07 or 2023.05.

I will check compatibility in a few days and report here if the plugin can be used with the currently published version.

Also, please keep in mind that due to the 3rd party restrictions, currently, the new approach requires .NET 6.x installed on the agent.

@OsirisTerje
Copy link
Member

@mchechulnikov That sounds awesome!

I assume the 2023.09 will be released some time in September?
Can you provide a link to the release notes when it is out ?

@mchechulnikov
Copy link

mchechulnikov commented Aug 16, 2023

@mchechulnikov That sounds awesome!

I assume the 2023.09 will be released some time in September? Can you provide a link to the release notes when it is out ?

@OsirisTerje Sure, I'll share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Tracked here, but must be fixed in external tool
Projects
None yet
Development

No branches or pull requests

3 participants