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

Fail-fast test execution if any test fails (OSOE-675) #309

Closed
Piedone opened this issue Aug 22, 2023 · 8 comments · Fixed by #325
Closed

Fail-fast test execution if any test fails (OSOE-675) #309

Piedone opened this issue Aug 22, 2023 · 8 comments · Fixed by #325
Assignees
Labels
enhancement New feature or request

Comments

@Piedone
Copy link
Member

Piedone commented Aug 22, 2023

When you break something that causes multiple UI tests to fail, then it can take a lot of time until test execution finishes, since all failing tests will be retried until the configured try count. Instead, we could have an option to fail the whole test execution immediately if any tests fail.

XUnit has the stopOnFail config for this. Instead of a config file, a command line option might be better, but for that we need to upgrade xunit.runner.visualstudio to the upcoming v2.5.1 in all UI test projects.

This might need an option in Lombiq GitHub Actions as well. Setting it in the config file is no issue either, since we already do that in Set-XUnitMaxParallelThreads.ps1.

Jira issue

@Piedone Piedone added the enhancement New feature or request label Aug 22, 2023
@github-actions github-actions bot changed the title Fail-fast test execution if any test fails Fail-fast test execution if any test fails (OSOE-675) Aug 22, 2023
@BenedekFarkas
Copy link
Member

I think it's a great idea! Is it possible to further fine-tune how/when the test execution is terminated?

The reason I'm asking: a single breaking change can cause multiple tests (usually in the same test suite, but not necessarily) to fail.
The way I see it now, the optimal approach would be that if a test fails in a given namespace, then we should still the run the other tests in the same namespace, but without a retry (since those breaking most probably has the same root cause, although some flakiness might still appear). This way we don't create a long loop of fixing the same problem in multiple tests over multiple builds (which would also include rebuilding the whole solution in every iteration), although the dev should run the tests locally in this case.

@Piedone
Copy link
Member Author

Piedone commented Aug 23, 2023

I think there are (or were at least) extension points for canceling tests (or test execution as a whole) that I read about while digging up stopOnFail but can't find anything now. If indeed, we could use that to implement arbitrary logic.

I'd be cautious with disabling retries, since that can bring up false positives with flaky tests too.

The namespace might not be a suitable way to correlate tests, but we could at least look into the current test collection (which by default consists of tests in the same class but with attributes can contain other tests too).

@BenedekFarkas
Copy link
Member

Sure, whatever works, I was just focusing on the concept, not the implementation details, but namespace isn't always suitable indeed.

@tteguayco tteguayco self-assigned this Oct 31, 2023
@tteguayco
Copy link
Contributor

XUnit has the stopOnFail config for this. Instead of a config file, a command line option might be better, but for that xunit/visualstudio.xunit#189 in all UI test projects.

When we refer to "all UI test projects", do we mean the ones here? i.e. Lombiq.Tests.UI.AppExtensions, Lombiq.Tests.UI.Samples, etc. I've seen that those projects that include xunit or xunit.runner.visualstudio as dependencies are already targeting version 2.5.1.

Also, in what context do we need a command line option for the stopOnFail flag?

Lastly, regarding:

This might need an option in Lombiq GitHub Actions as well. Setting it in the config file is no issue either, since we already do that in Set-XUnitMaxParallelThreads.ps1.

Can I get some more contexts here? Like where we would expect that new option to be included or used.

Also, the mentioned script Set-XUnitMaxParallelThreads.ps1 doesn't seem to live in that GHA repo. Is it somewhere else?

@Piedone
Copy link
Member Author

Piedone commented Nov 6, 2023

We mean all UI test (or other test) projects in the OSOCE solution, but then an upgrade is not necessary, because indeed since then, all affected projects are updated.

If the implementation of this issue will affect how tests are launched (as opposed to changes to this project), then that'll need to be part of the test-dotnet action of ours, most possibly here. Set-XUnitMaxParallelThreads.ps1 is also there.

But! Since now this seems to be possible with just an xUnit config, we don't need anything for it in this project, nor Lombiq GitHub Actions. So, please try this xunit.runner.json in Lombiq.OSOCE.Tests.UI and Lombiq.Tests.UI.Samples, the only projects where we'd need this in OSOCE, and check if it indeed works as advertised:

{
  "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
  "parallelizeAssembly": false,
  "parallelizeTestCollections": true,
  "maxParallelThreads": 3,
  "stopOnFail": true
}

If it does, then nothing else to do apart from adding a note about it under https://github.com/Lombiq/UI-Testing-Toolbox/blob/dev/Lombiq.Tests.UI/Docs/Configuration.md#external-configuration.

@tteguayco
Copy link
Contributor

The option stopOnFail seems to be ignored for me. I just opened xunit/visualstudio.xunit#392.

@Piedone
Copy link
Member Author

Piedone commented Nov 12, 2023

BTW we do need "parallelizeTestCollections": true in this project, so the xUnit bug is not an issue for us.

@Piedone
Copy link
Member Author

Piedone commented Nov 17, 2023

As I've written above, docs are also necessary, as is the same config for the samples UI test project (the other UI test project in OSOCE).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants