Add disable parallelization for collection sequential

[LeafShi1]

related #11227

Proposed changes

• Add [CollectionDefinition("Sequential", DisableParallelization = true)] for collection Sequential

  Microsoft Reviewers: Open in CodeFlow

[LeafShi1]

Adopt Paul's comment in commit 4d471144

[LeafShi1]

@paul1956 Please help review this PR

[paul1956]

@LeafShi1 @Tanya-Solyanik I think there may be two issues here. In my experience without

[CollectionDefinition("Sequential", DisableParallelization = true)]

my VB tests #11215 failed randomly with same error others reported. With above Attribute all my new tests in VisualBasic Runitme (Written in both C# and VB) in separate projects pass when I run just the tests I changed against the VB runtime. But when I run all tests, it sometimes fails with the Clipboard being corrupted between setting and reading.

I believe the issue is some existing tests (not part of Visual Basic Runtime) use the Clipboard (at least the ones being disabled as Flakey) plus possibly more need to be added to the same collection. I don't know if doing that removes the requirement to add the CollectionDefination Attribute, all the searching seems to say you need collection name and collection definition.

My other point was is the Clipboard the only shared resource in the repo? If it is than using "Sequential" as the name is fine, but if there are other similar resources they should have their own collection name so that they can run in parallel with tests using Clipboard.

[Tanya-Solyanik]

[CollectionDefinition("Sequential", DisableParallelization = true)]

is an incorrect use of this attribute according to xunit docs. Your other 2 points are valid but shoudl be addressed in a different PR

1. Tests that exercise clipboard directly (ClipboardTests class) as well as all other tests that use clipboard indirectly(for example copy/paste in some datagrid tests) should be in a single collection
2. Other tests that use global resources don't have to be in the same collection as the clipboard tests are.

[paul1956]

@Tanya-Solyanik

1. Tests that exercise clipboard directly (ClipboardTests class) as well as all other tests that use clipboard indirectly(for example copy/paste in some datagrid tests) should be in a single collection

There are also and will be additional Clipboard Proxy tests in VB runtime that depend on Clipboard.

Whatever is decided for [CollectionDefinition("Sequential", DisableParallelization = true)] I will just follow.

Its interesting that CursorTestsCollection uses both [Collection(nameof(CursorTestsCollection))] [CollectionDefinition(nameof(CursorTestsCollection), DisableParallelization = true)]

For Clipboard the nameof will not work because multiple different classes use Clipboard

Without DisableParallelization With

[paul1956]

There are many different test classes in different projects and namespaces that use clipboard that is why including the second attribute is needed unless you want all control test mixed into one class. Sent from my iPhoneI apologize for any typos Siri might have made. Tanya Solyanik @.***> wrote: @Tanya-Solyanik commented on this pull request.

In src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasic/MyServices/ClipboardProxyTests.cs:

@@ -9,6 +9,7 @@ namespace Microsoft.VisualBasic.MyServices.Tests;

[Collection("Sequential")] +[CollectionDefinition("Sequential", DisableParallelization = true)]

This attribute should be placed on the "collection definition class" - https://xunit.net/docs/running-tests-in-parallel so it would apply only to this class... Ideally we would want all clipboard tests across the assembly in a single collection.

[paul1956]

The issue is tests are in different classes. Attribute [CollectionDefinition(DisableParallelization = true)] doesn't prevent parallel execution between class · Issue #1999 · xunit/xunitgithub.comSent from my iPhoneI apologize for any typos Siri might have made.(503) 803-6077On Jun 14, 2024, at 11:22 AM, Tanya Solyanik @.***> wrote: @Tanya-Solyanik commented on this pull request.

In src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasic/MyServices/ClipboardProxyTests.cs:

@@ -9,6 +9,7 @@ namespace Microsoft.VisualBasic.MyServices.Tests;

[Collection("Sequential")] +[CollectionDefinition("Sequential", DisableParallelization = true)]

I see, xunit runs all tests within a collection, test class is always a collection and tests within a class run sequentially. We use the [Collection()] attribute to extend the set of tests that run sequentially. So [CollectionDefinition("Sequential", DisableParallelization = true)] is not needed.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

[paul1956]

@LeafShi1 @Tanya-Solyanik the problem with moving “Collection” attribute to a class is the Clipboard is shared by many test classes in multiple projects. Logically tests are not groups by those that use Clipboard and those that don’t, they are grouped by control or class the objects they are testing are in. Does it make sense to group the VB Clipboard Proxy tests with TextBox tests I don’t think so.

[LeafShi1]

And according to the document, add attribute [CollectionDefinition(DisableParallelization = true)] only affect current assembly