Skip to content

Tests: add helper Stress attribute for stress testing - #18452

Merged
T-Gro merged 16 commits into
dotnet:mainfrom
majocha:tests-repeat
Apr 10, 2025
Merged

Tests: add helper Stress attribute for stress testing#18452
T-Gro merged 16 commits into
dotnet:mainfrom
majocha:tests-repeat

Conversation

@majocha

@majochamajocha commented Apr 7, 2025

Copy link
Copy Markdown
Contributor

This is useful for locally stress testing cases that are flaky in parallel execution.

Example usage: replace [<Fact>] with [<Theory; Stress(Count = 1000)>]

Unlike VS "Run until failure" feature which runs sequentially, this will start the test simultaneously many times and run in parallel, stressing the threadpool.

It can be used to locally track down bugs in flaky tests.

@github-actions

Copy link
Copy Markdown
Contributor

✅ No release notes required

@majocha
majocha marked this pull request as ready for review April 7, 2025 10:15
@majocha
majocha requested a review from a team as a code ownerApril 7, 2025 10:15
Comment threadtests/FSharp.Compiler.ComponentTests/CompilerService/AsyncMemoize.fs Outdated
@psfinaki

Copy link
Copy Markdown
Contributor

Good stuff. I think VS allows for "run until failure" but not sure if it stressing the thread pool this much, and also it's not x-plat.

Would you mind adding a line about that to the testing guide? :)

…ize.fs
Co-authored-by: Petr <psfinaki@users.noreply.github.com>
@majocha

Copy link
Copy Markdown
ContributorAuthor

Good stuff. I think VS allows for "run until failure" but not sure if it stressing the thread pool

The difference is VS "run until failure" will execute sequentially if we select only one test case. For example it couldn't reproduce the failure I linked above, while with Repeat it repro'd immediately.

Hmm, now I think the naming could be better.

@majocha

Copy link
Copy Markdown
ContributorAuthor

I'm not sure if this should to TESTGUIDE or DEVGUIDE? There was a section already to which I added.

@psfinakipsfinaki left a comment

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.

I think this is good enough for now :) Thanks!

@psfinaki

Copy link
Copy Markdown
Contributor

Speaking of naming, I think "Stress" would be even better than "Repeat".

@majochamajocha changed the title Tests: add helper Repeat attribute for stress testingTests: add helper Stress attribute for stress testingApr 7, 2025
KevinRansom
KevinRansom previously requested changes Apr 7, 2025

@KevinRansomKevinRansom left a comment

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.

I like this a lot, it looks super useful.

Can you add a test case where this attribute is used and that we verify it passes when al iterations succeed, and fails when one pass doesn't succeed.

@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingApr 7, 2025
@majocha

Copy link
Copy Markdown
ContributorAuthor

Makes sense! But now I'm in decision paralysis where to put such test 😄. This will be a test of test utilities, so it doesn't belong anywhere...

@KevinRansom

Copy link
Copy Markdown
Contributor

Makes sense! But now I'm in decision paralysis where to put such test 😄. This will be a test of test utilities, so it doesn't belong anywhere...

Well, we can always start a testutilities test assembly. With this as the first test, hereafter, we will have no excuse for not testing the hacks we add to the test framework.

@KevinRansom

Copy link
Copy Markdown
Contributor

Makes sense! But now I'm in decision paralysis where to put such test 😄. This will be a test of test utilities, so it doesn't belong anywhere...

Well, we can always start a testutilities test assembly. With this as the first test, hereafter, we will have no excuse for not testing the hacks we add to the test framework.

I'm guessing many of the smoketests are only smoketests because they are testing that the test framework does something interesting.

@majocha

Copy link
Copy Markdown
ContributorAuthor

Would it be ok to just unlock existing FSharp.Test.Utilities project as a test container?
Currently, because it does reference xUnit, it has some extra stuff like
<IsTestProject>false</IsTestProject>
and
<ProjectCapability Remove="TestContainer" />
It looks convenient to place the relevant tests there instead of creating new project.

Comment threadtests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj
@T-Gro

T-Gro commented Apr 8, 2025

Copy link
Copy Markdown
Member

Would it be ok to just unlock existing FSharp.Test.Utilities project as a test container? Currently, because it does reference xUnit, it has some extra stuff like <IsTestProject>false</IsTestProject> and <ProjectCapability Remove="TestContainer" /> It looks convenient to place the relevant tests there instead of creating new project.

I would do it like that as well, keep the tests close to those utilities.

@majocha

Copy link
Copy Markdown
ContributorAuthor

Tests added :)

Comment threadtests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@majocha@psfinaki@KevinRansom@T-Gro