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

Task: Split J2N.Tests.xUnit into separate projects per collection to improve performance #120

Merged
merged 37 commits into from
Nov 9, 2024

Conversation

NightOwl888
Copy link
Owner

Fixes #109

This refactors the test projects to put NUnit and XUnit into their own directory with their own dependencies. The single J2N.Tests.xUnit project was split into multiple projects:

Test Projects

  • J2N.Collections.Concurrent.LurchTable.Tests
  • J2N.Collections.Generic.Dictionary.Tests
  • J2N.Collections.Generic.Extensions.Tests
  • J2N.Collections.Generic.HashSet.Tests
  • J2N.Collections.Generic.LinkedDictionary.Tests
  • J2N.Collections.Generic.LinkedHashSet.Tests
  • J2N.Collections.Generic.List.Tests
  • J2N.Collections.Generic.SortedDictionary.Tests
  • J2N.Collections.Generic.SortedSet.Tests
  • J2N.Collections.ObjectModel.Tests
  • J2N.Runtime.Tests

Test Infrastructure Projects

  • J2N.TestCommon - Base classes for collection tests that are shared among collection Test projects
  • J2N.TestUtilities - Shared infrastructure between XUnit and NUnit, such as platform detection
  • J2N.TestUtilities.Xunit - Extensions to Xunit for use in the Microsoft-derived tests

The infrastructure for NUnit (J2N.TestFramework) was also moved from src to tests/NUnit along side J2N.Tests, but neither of them have been split (we may consider this because there are 40,000 tests).

Optimization

There were some optimizations done to make the Azure DevOps pipeline faster.

  • The run-tests-on-os.yml template was changed to only allow a single target framework. This allows us to pass the target framework to publish-test-results-for-test-projects.yml so we don't spend time cycling through all of the target frameworks (the publish-test-results-for-target-frameworks.yml template was removed).
  • The tests were reordered to queue the slowest test projects first to make maximum use of parallel jobs.
  • The parameter validation has been disabled unless the System.Debug environment variable is true.

Publishing

This also changes the publishing to put each project into its own directory rather than publishing the entire solution to a single directory, which is no longer supported.

Results

Unfortunately, this didn't make as big of an impact as was hoped, shaving about 5 minutes off of a build/test run.

This could be significantly improved by uploading the test results through the REST API from a background job instead of waiting for all of the results to be uploaded using the PublishTestResults@2 task. I looked into this, but it looks like it will require us to:

  • Parse the result header from the TRX (XML) file
  • Call the REST API to create a test result record (with a title built from the header info)
  • Parse the test results from the TRX (XML) file in batches
  • Call the REST API in batches to upload the test results to the result record

Maybe something this complex should be put into a dotnet tool or a Powershell module, both of which could be reused in all of our projects. Last I checked Microsoft used to maintain such a tool for TFS, but the project has been abandoned.

…ectory-level configuration across projects that apply to a given test framework.
…ectories and reorganized warning suppressions so they apply globally, to NUnit, or Xunit tests.
…TestUtilities.Xunit, J2N.TestFramework, and J2N.TestCommon so the build script will skip them when running tests
…es.Xunit to the tests/Xunit folder and J2N.TestUtilities to the tests folder from the src/ folder
…ince it applies to all projects in this directory.
…mmon.References.targets to both NUnit and XUnit folders to simplify adding new test projects
…we don't need to check for all of the target frameworks that are not being currently tested
…t rather than build artifact"

This reverts commit 4491cd9.
@NightOwl888 NightOwl888 added this to the 2.2 milestone Nov 9, 2024
@NightOwl888 NightOwl888 merged commit 4746469 into main Nov 9, 2024
22 checks passed
@NightOwl888 NightOwl888 added the notes:improvement An enhancement to an existing feature label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:improvement An enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task: Split J2N.Tests.xUnit into separate projects per collection to improve performance
1 participant