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

feat(regTests): upload only failed test logs on ci and clean up logging #2547

Merged
merged 13 commits into from
Feb 21, 2024

Conversation

kostasrim
Copy link
Contributor

  • upload only failed test logs
  • remove printing log names for passed tests

@kostasrim kostasrim self-assigned this Feb 5, 2024
@kostasrim
Copy link
Contributor Author

@@ -87,10 +87,6 @@ runs:
TIMEDOUT_STEP_1: ${{ steps.first.outputs.TIMEDOUT }}
TIMEDOUT_STEP_2: ${{ steps.second.outputs.TIMEDOUT }}
run: |
echo "🪵🪵🪵🪵🪵🪵 Latest log before timeout 🪵🪵🪵🪵🪵🪵\n\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this. Although it can be helpful in certain cases it's also annoying because a lot of the tests do not write to a single log, so printing only one them is not that useful and is also noisy. Since now we upload only the failed logs, it should be straightforward to download + open them.

If you have strong objections against this I am happy to revert.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not understand, sorry.
The change in this PR is that you dont print the failed test logs but just upload them?

In here is see you removed a section of last log before timeout. So do we upload logs for timeout tests?

Copy link
Contributor Author

@kostasrim kostasrim Feb 8, 2024

Choose a reason for hiding this comment

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

The change in this PR is that you dont print the failed test logs but just upload them?

Nope, I removed printing the last log of the last failed test (since the last test can contain many logs) because it was not really useful most of the time.

In addition, for each test run, we used to print the corresponding logs. This was annoying because we are only interested in the logs of the tests that failed. So I removed that as well and now we only print the name of the logs of the failed tests. See here for an example: https://github.com/dragonflydb/dragonfly/actions/runs/7798539464/job/21267436332#step:6:178 (other tests do not print this).

Moreover, previously we also uploaded all the logs (both from the failed and successful tests). This was again annoying so we now only upload the logs of the failed tests.

@kostasrim kostasrim changed the title (WIP do not review) feat(regTests): upload only failed test logs on ci and clean up logging feat(regTests): upload only failed test logs on ci and clean up logging Feb 6, 2024
@kostasrim kostasrim marked this pull request as ready for review February 6, 2024 11:27
@dranikpg
Copy link
Contributor

Can we maybe add this https://stackoverflow.com/questions/27884404/printing-test-execution-times-and-pinning-down-slow-tests-with-py-test?

To see the 20, 30 slowest tests?

dranikpg
dranikpg previously approved these changes Feb 14, 2024
Copy link
Contributor

@dranikpg dranikpg left a comment

Choose a reason for hiding this comment

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

Maybe we also don't need the port logged for every test

@kostasrim kostasrim merged commit f321567 into main Feb 21, 2024
10 checks passed
@kostasrim kostasrim deleted the simplify_ci branch February 21, 2024 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants