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

introduce JUnit tag "flaky", to exclude tests from running. #5231

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

soloturn
Copy link
Contributor

@soloturn soloturn commented Apr 1, 2024

ExampleTest.testClientCreation fails without real reason, mark it flaky at the moment. permit this tag in engine-tests/build.gadle.kts to separate test runs for normal integration tests, and ones which are not stable on github, for currently unknown reasons. normally of course it would be better to fix these tests - but accepting pull requests is coupled on tests running through. now two commands instead of one run all integration tests:

gradle integrationTest
gradle integrationTestFlaky

please do not lightheartedly mark tests as flaky.

@soloturn soloturn force-pushed the qa/flaky-integration branch 2 times, most recently from 915f87a to b4ff9fa Compare April 1, 2024 00:32
ExampleTest.testClientCreation fails without real reason, mark it flaky
at the moment. permit this tag in engine-tests/build.gadle.kts to
separate test runs for normal integration tests, and ones which are
not stable on github, for currently unknown reasons. normally of course
it would be better to fix these tests - but accepting pull requests
is coupled on tests running through. now two commands instead of one
run all integration tests:

    gradle integrationTest
    gradle integrationTestFlaky

please do not lightheartedly mark tests as flaky.
Copy link
Member

@jdrueckert jdrueckert left a comment

Choose a reason for hiding this comment

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

In general, this looks good to me.
I made a few suggestions based on my personal preferences of wording and expression, they are not blocking though.

I split the CI stage using the new gradle task you introduced which works nicely 👍
As you can see the tests are executed by the stage matching their tagging:
image

org.terasology.engine.integrationenvironment.ExampleTest.testClientConnection() failed in the last CI run. This test is one of the other flaky tests in ExampleTest that I mentioned in the last sync. We probably want to mark at least that one "flaky", too.

engine-tests/build.gradle.kts Outdated Show resolved Hide resolved
engine-tests/build.gradle.kts Outdated Show resolved Hide resolved
engine-tests/build.gradle.kts Outdated Show resolved Hide resolved
@jdrueckert
Copy link
Member

image

The delay manager test fails so rarely that I wouldn't mark that one flaky, but the ExampleTests fail more frequently, so let's mark those flaky. Amusingly the one already marked flaky didn't fail this time 😅

@soloturn
Copy link
Contributor Author

soloturn commented Apr 3, 2024

ok, committed your suggestions and squashed into one. i am understanding correct, we still see the cause of any failed integration test?

@soloturn soloturn requested a review from jdrueckert April 3, 2024 15:51
result of flaky test will be displayed in also github, result of failing
normal test is viewed in jenkins.
BenjaminAmos
BenjaminAmos previously approved these changes Apr 4, 2024
Copy link
Contributor

@BenjaminAmos BenjaminAmos left a comment

Choose a reason for hiding this comment

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

There shouldn't be any flaky tests, however, those flaking tests have little benefit and the failures have been very disruptive. This is likely the most practical path forward right now.

engine-tests/build.gradle.kts Outdated Show resolved Hide resolved
Co-authored-by: BenjaminAmos <24301287+BenjaminAmos@users.noreply.github.com>
@soloturn soloturn merged commit d0c6e86 into develop Apr 5, 2024
10 checks passed
@soloturn soloturn deleted the qa/flaky-integration branch April 5, 2024 22:57
@jdrueckert
Copy link
Member

i am understanding correct, we still see the cause of any failed integration test?

in the sense that test failures are still reported including stack trace etc.? yes
but they shouldn't fail the pipeline anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants