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

[NO TICKET] Improving logging of Azure e2e test and increase WDS timeout #2707

Merged
merged 9 commits into from
Jan 22, 2024

Conversation

cahrens
Copy link
Contributor

@cahrens cahrens commented Jan 19, 2024

I have noticed that the message being passed to withClue doesn't appear in the test execution log in the case of these tests. Therefore I created a different method that logs the message (instead of just appending it).

And I am increasing the timeout for WDS to be out of PROVISIONING to 15 minutes; it's not clear if this will help at all with the sporadic "over 10 minute" failures we have seen (it's possible that WDS is in a state will it will never succeed), but we can at least get more data.

I have run this workflow, both with a short timeout to ensure that the log message does indeed print in the log (see screenshot below), and with the final version to ensure everything passes.
image

@cahrens
Copy link
Contributor Author

cahrens commented Jan 22, 2024

jenkins retest

@cahrens cahrens changed the title E2e update [NO TICKET] Improving logging of Azure e2e test and increase WDS timeout Jan 22, 2024
@cahrens cahrens marked this pull request as ready for review January 22, 2024 18:26
@cahrens cahrens requested review from a team, marctalbott and aherbst-broad and removed request for a team January 22, 2024 18:26
Copy link
Contributor

@aherbst-broad aherbst-broad left a comment

Choose a reason for hiding this comment

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

Very curious why withClue isn't doing what we want here, but this seems like a reasonable compromise.

@cahrens
Copy link
Contributor Author

cahrens commented Jan 22, 2024

@aherbst-broad from what I read, withClue appends the message to the stack trace information. But I don't think we see the stack trace from a test failure in the execution log in these tests in the same way that we would if the test was being executed as a normal unittest.

Also, logging it in this way keeps the message in synchronous order with the other things that are being printed out.

@cahrens
Copy link
Contributor Author

cahrens commented Jan 22, 2024

@aherbst-broad I think it may be because we catch the timeout exception and attempt workspace deletion (which then fails)… so the timeout isn't what causes the test to fail. Perhaps withClue doesn't really need to be replaced throughout, but I do like having the messaging appear synchronously.

Example where we don't see "WDS did not become deletable within the timeout period of…": https://github.com/broadinstitute/terra-github-workflows/actions/runs/7565178023/job/20600517481

@cahrens cahrens merged commit 8134641 into develop Jan 22, 2024
20 checks passed
@cahrens cahrens deleted the e2e_update branch January 22, 2024 21:28
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.

3 participants