Skip to content

desktop_tester.py: some random improvements to help with debugging failed runs #1027

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 19, 2022
Merged

desktop_tester.py: some random improvements to help with debugging failed runs #1027

merged 4 commits into from
Jul 19, 2022

Conversation

dconeybe
Copy link
Contributor

This PR makes a bunch of random improvements to scripts/gha/desktop_tester.py. These improvements primarily help with making sense of failed test runs.

Here is a non-exhaustive list of changes:

  • Use shlex.split() instead of str.split() on the --cmd_line argument, to intelligently handle quoted strings with spaces in them.
  • Avoid initializing result = None and instead leverage Python's try/except/else structure.
  • Log the full command line of the test execution.
  • Print the elapsed time of the test execution.
  • Specify errors="replace" to subprocess.run() so that any decoding errors of stdout/stderr won't cause a crash.

@dconeybe dconeybe added the skip-release-notes Skip release notes check label Jul 15, 2022
@dconeybe dconeybe self-assigned this Jul 15, 2022
@dconeybe dconeybe requested a review from sunmou99 July 15, 2022 18:33
@sunmou99 sunmou99 added the tests-requested: quick Trigger a quick set of integration tests. label Jul 15, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Jul 15, 2022
@github-actions
Copy link

github-actions bot commented Jul 15, 2022 •
edited
Loading

✅  Integration test succeeded!

Requested by @dconeybe on commit 78917c3
Last updated: Mon Jul 18 21:23 PDT 2022
View integration test log & download artifacts

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Jul 15, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 15, 2022
@dconeybe
Revert "Combine environment setting steps into one step with shell sc…
9330d59 
…ript (#1010)"

This reverts commit 5ccf419.

This is a potential fix for the test failures of `test-desktop-windows-latest-x64-openssl`
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jul 16, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Jul 16, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 16, 2022
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jul 18, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Jul 18, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 18, 2022
@dconeybe
Copy link
Contributor Author

FYI The one failing integration tests, test-desktop-windows-latest-x64-openssl can be fixed by rolling back #1010, which re-arranged a bunch of the GitHub Actions logic.

sunmou99
sunmou99 approved these changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@sunmou99 sunmou99 left a comment

Choose a reason for hiding this comment

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

LGTM

@dconeybe dconeybe removed the tests: failed This PR's integration tests failed. label Jul 18, 2022
dconeybe added a commit that referenced this pull request Jul 18, 2022
@dconeybe
Revert changes to desktop_tester.py as they are now incorporated into #…
5ff28e2 
…1027
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Jul 18, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Jul 18, 2022
@dconeybe dconeybe enabled auto-merge (squash) July 19, 2022 01:42
@github-actions github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Jul 19, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 19, 2022
@dconeybe dconeybe merged commit 78917c3 into main Jul 19, 2022
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests: succeeded This PR's integration tests succeeded. labels Jul 19, 2022
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 19, 2022
@dconeybe dconeybe deleted the dconeybe/DesktopTesterPyRandomImprovements branch July 19, 2022 14:38
@firebase firebase locked and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sunmou99 sunmou99 sunmou99 approved these changes

Assignees

@dconeybe dconeybe

@sunmou99 sunmou99

Labels
skip-release-notes Skip release notes check tests: succeeded This PR's integration tests succeeded.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dconeybe @sunmou99