SubmoduleStatusProviderTests extremely slow

[RussKie]

Current behaviour

Running tests under VS I've observed SubmoduleStatusProviderTests tests to take in excess of 7 minutes to complete.

Expected behaviour

Tests must complete faster.

[gerhardol]

1,6 minutes for me. Six times execution time, virus that slows Git down?
Every test takes 4-5s, also the simple setup tests so most of the test time is likely to create repos etc.
The tests could be changed to execute in one testcase.

Not fast, but it actually test something.

[mstv]

Run all tests does not work anymore. It builds everything and does not run any test.

The SubmoduleStatusProviderTests took 6.6 minutes, too, with high CPU load by the anti-virus virus. I cannot turn it off on this machine.

[RussKie]

My stuff is run on a VM in Parallels, the way I always had it. No AV other than the standard Windows Defender. Since I refactored the build infra, I always run builds and tests from a cli. I just started running them again in a VS porting to .NET Core. I'm going to check the master for issues running tests from a VS.

[sharwell]

I can do a profiling pass to see what's taking time here.

[sharwell]

So the general approach to use here leverages two things:

1. Use LibGit2Sharp for repository initialization, because it's faster than the Git executable. This is not currently possible for this test because there is no API to add a submodule (SubmoduleCollection.Add() using clone libgit2/libgit2sharp#1232, Add SubmoduleCollection.Add libgit2/libgit2sharp#1501).
2. Avoid deleting and recreating the entire repository between each test. Instead, reset the repository to its initial state. You're basically looking for something like ReferenceRepository.Reset(), except specific to the types of changes made to a repository by this test class.

Realistically you should be able to shave about 70% off the test time by avoiding some calls to git for all tests except the first.

[vbjay]

What about extracting zipped repo to temp folder to have it at a known state. Then do things to the repo. Could also use a bundle. Add a remote to the extracted bundle. Then just code the reset to do a reset and maybe clean of working directory to a specific sha. Could even store different sha's for different initial states.

[sharwell]

I'm opposed to extracting a zipped repo if it can be avoided any other way. Such a repository would not be reviewable if we need to make any changes to its form. The ReferenceRepository approach is essentially the same result, except its much smaller and the generated repository state can be determined by reviewing the code. It's similar to the difference between storing a raw SQL database and storing a SQL script that would create the database.

[vbjay]

Why I also suggested a bundle. https://git-scm.com/docs/git-bundle

[sharwell]

😕 I don't understand how a bundle improves the ability to review changes. Isn't it still a binary file?

[vbjay]

It's a repo. You can add a remote to it. Fetch from it, clone it, compare branches.....

[vbjay]

Basically, we have a test setup process before tests run that generates or copies the bundle to artifacts folder. Then each test that needs initial repo state can either clone from bundle into testrepos\test name or id\ or initial setup clones from bundle once and setup for each test resets repo to x Sha. The bundle is your initial repo with initial state. It could have initial work at a known state. Then each test can test against the results of doing x action to local repo. This would allow us to do whatever to the local repo and always reset to origin\initial for example.

[RussKie]

Whatever process we chose it has to be immune to concurrent access and race conditions as tests often run in parallel.

[sharwell]

tests often run in parallel

xunit doesn't run tests in the same class in parallel. Does nunit?

[RussKie]

I don't think so.

[gerhardol]

The solution to improve the tests I can see is a hack to check that [SetUp] only executes once for a fixture.

GE uses BeforeTest to setup the thread handling etc, but OneTimeSetup runs before that.
https://medium.com/a-layman/test-with-nunit-create-a-test-project-and-realize-the-execution-order-18c11b636b0c

xunit doesn't run tests in the same class in parallel. Does nunit?

Seem to be possible, even if we have not set that up.
NUnit tests can be run in parallel, so changing the resource usage will require use of attributes
https://docs.nunit.org/articles/nunit/technical-notes/usage/Framework-Parallel-Test-Execution.html

For #8334 trying to improve the tests, I tried to setup according to Parallel class with non-parallel methods in NUnit documentation

[mstv]

Parallel test execution is no option ATM. There are too many singletons, e.g. AppSettings.

[RussKie]

i looked at this problem last night, and a had a partial success. about half of tests failed because they expected a clean state. we need to reset submodules to first commit. I need help with that.

[gerhardol]

@RussKie see the PR for a solution

For parallel: I just tried to setup the attributes correctly if parallel is enabled