Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

make sure BindDuringTcpWait_Succeeds does not run with other tests#40958

Merged
wfurt merged 3 commits into
dotnet:masterfrom
wfurt:no_parallel_40475
Sep 12, 2019
Merged

make sure BindDuringTcpWait_Succeeds does not run with other tests#40958
wfurt merged 3 commits into
dotnet:masterfrom
wfurt:no_parallel_40475

Conversation

@wfurt

@wfurt wfurt commented Sep 9, 2019

Copy link
Copy Markdown
Member

I check run log and the test runs at the end as @bradwilson described. This should avoid conflicts with other tests.

Right now it is only one test but I put CollectionDefinition to HelperBase so it is easy to add more as we find mode cases like this.

      System.Net.Sockets.Tests.NoParallelTests.BindDuringTcpWait_Succeeds [STARTING]
      System.Net.Sockets.Tests.NoParallelTests.BindDuringTcpWait_Succeeds [FINISHED] Time: 0.0012822s
    Finished:    System.Net.Sockets.Tests
  === TEST EXECUTION SUMMARY ===
     System.Net.Sockets.Tests  Total: 920, Errors: 0, Failed: 0, Skipped: 7, Time: 6.013s

fixes #40475

@wfurt wfurt requested a review from a team September 9, 2019 22:46
@wfurt wfurt self-assigned this Sep 9, 2019
@wfurt wfurt added the test bug Problem in test source code (most likely) label Sep 9, 2019
Comment thread src/System.Net.Sockets/tests/FunctionalTests/SocketOptionNameTest.cs Outdated
{

// Define test collection for tests to avoid all other tests.
[CollectionDefinition("NoParallelTests", DisableParallelization = true)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a useful pattern to document more broadly in our "how to write unit tests for CoreFx" documentation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wfurt I'm not sure this does what you want it to.
My understanding is this means that tests in this collection do not run in parallel.
By default the collections are determined by tests grouped in the same class.
So I think the effect of this change is BindDuringTcpWait_Succeeds runs in parallel with more tests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trick comes from DisableParallelization = true @tmds. I was confused as well by normal documentation but the interesting part comes from https://xunit.net/releases/2.3-beta5

Added the ability to disable cross-collection parallelization for individual test collections, via the test collection definition (by setting [CollectionDefinition(DisableParallelization = true)]). Parallel-capable test collections will be run first (in parallel), followed by parallel-disabled test collections (run sequentially).

I verified that behavior and observed that my class and test will be at the end. When DisableParallelization is removed new test is mingled inside other tests. https://xunit.net/docs/running-tests-in-parallel.html does not really explain how that parameter works.

As far as the documentation: I think that needs more work in general. My plan is to wait and verify that #40475 is truly fixed and then I can update testing notes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collection definition classes are intended to be empty classes. I'm not sure why you've attached this to some seemingly random existing class. Create a new empty class to attach this to instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I missed something It did not seems like I can attach [Collection] attribute to specific test @bradwilson. So I still needed class where test can live.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I was looking for way how to update running-tests-in-parallel.html but it does not seems it lives on github. I think the fact that DisableParallelization=true collections run at the end in sequence is crucial piece of information.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant specifically this line ([CollectionDefinition]), not the test ([Collection]).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. And thanks for the doc pointer @bradwilson.

@davidsh davidsh added this to the 5.0 milestone Sep 9, 2019
@wfurt wfurt merged commit b384d34 into dotnet:master Sep 12, 2019
@wfurt wfurt deleted the no_parallel_40475 branch September 12, 2019 18:48
ViktorHofer pushed a commit to ViktorHofer/corefx that referenced this pull request Sep 26, 2019
…otnet#40958)

* make sure BindDuringTcpWait_Succeeds does not run with other tests

* add partial to  NoParallelTests class

* add empty class to CollectionDefinition
stephentoub pushed a commit that referenced this pull request Sep 27, 2019
…40958)

* make sure BindDuringTcpWait_Succeeds does not run with other tests

* add partial to  NoParallelTests class

* add empty class to CollectionDefinition
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…otnet/corefx#40958)

* make sure BindDuringTcpWait_Succeeds does not run with other tests

* add partial to  NoParallelTests class

* add empty class to CollectionDefinition


Commit migrated from dotnet/corefx@b384d34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Net test bug Problem in test source code (most likely)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Net.Sockets.Tests.SocketOptionNameTest.BindDuringTcpWait_Succeeds failing in CI

5 participants