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

RATIS-2112. Improve repeat-test workflow #1112

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Improve repeat-test workflow:

  • Skip artifact upload if all iterations passed
  • Rename artifact using run number/ID
  • Fail fast if no tests were run (e.g. due to workflow triggered for non-existent test class)

https://issues.apache.org/jira/browse/RATIS-2112

How was this patch tested?

repeat-test run without failures:
https://github.com/adoroszlai/ratis/actions/runs/9486024280

repeat-test run with non-existent test class:
https://github.com/adoroszlai/ratis/actions/runs/9486291646

@adoroszlai adoroszlai added the CI label Jun 12, 2024
@adoroszlai adoroszlai self-assigned this Jun 12, 2024
@adoroszlai adoroszlai changed the title RATIS-2112. repeat-test should upload artifact only for failed split RATIS-2112. Improve repeat-test workflow Jun 12, 2024
Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

@adoroszlai , thanks for working on this! Just a minor comment inlined.

@@ -65,6 +65,12 @@ for i in $(seq 1 ${ITERATIONS}); do
fi

if [[ ${ITERATIONS} -gt 1 ]]; then
if ! grep -q "Running .*Test" "${REPORT_DIR}/output.log"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The pattern "Running .*Test" assume that the test name contains Test; e.g.

$grep "Running .*Test" ~/Downloads/result-3-9475609402-split-16/iteration4/output.log 
[INFO] Running org.apache.hadoop.fs.ozone.TestHSync

It may be better to grep "Tests run: [1-9]".

$grep "Tests run: [1-9]" ~/Downloads/result-3-9475609402-split-16/iteration4/output.log 
[ERROR] Tests run: 10, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 154.095 s <<< FAILURE! - in org.apache.hadoop.fs.ozone.TestHSync
[ERROR] Tests run: 10, Failures: 0, Errors: 1, Skipped: 0

Then, it will work for any test names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @szetszwo for the review.

Tests run: is not printed if there is a fork timeout (Ozone used to have a similar pattern before HDDS-10545).

[INFO] Running org.apache.hadoop.ozone.client.rpc.TestECKeyOutputStreamWithZeroCopy
[INFO] 
[INFO] Results:
...
... There was a timeout or other error in the fork

While this workflow allows any test to be run, Ratis limits test naming by:

ratis/pom.xml

Lines 647 to 649 in dac27e4

<includes>
<include>**/Test*.java</include>
</includes>

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thanks for the info.

Copy link
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

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

+1 the change looks good.

@@ -65,6 +65,12 @@ for i in $(seq 1 ${ITERATIONS}); do
fi

if [[ ${ITERATIONS} -gt 1 ]]; then
if ! grep -q "Running .*Test" "${REPORT_DIR}/output.log"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Thanks for the info.

@adoroszlai adoroszlai merged commit e540e46 into apache:master Jun 13, 2024
12 checks passed
@adoroszlai adoroszlai deleted the RATIS-2112 branch June 13, 2024 06:26
@adoroszlai
Copy link
Contributor Author

Thanks @szetszwo for the review.

szetszwo pushed a commit to szetszwo/ratis that referenced this pull request Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants