SG-43825 Improve pipeline stability#449
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #449 +/- ##
==========================================
+ Coverage 78.66% 78.76% +0.10%
==========================================
Files 7 7
Lines 1842 1851 +9
==========================================
+ Hits 1449 1458 +9
Misses 393 393
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5350717 to
d3146da
Compare
|
|
||
| data = {"code": cls.config.asset_code, "project": cls.project} | ||
| keys = ["code"] | ||
| keys = ["code", "project"] |
There was a problem hiding this comment.
Since we are now creating many projects with entities of the same name, we must scope the search to the ephemeral project.
There was a problem hiding this comment.
Pull request overview
This PR aims to improve Azure Pipelines stability for the test matrix by isolating concurrent jobs from shared live-state (projects/entities) and reducing failures from transient test flakiness/timeouts.
Changes:
- Introduces per-job ephemeral project naming (scoped to Build + OS + Python version) and adds an unconditional cleanup step to remove the project at job end.
- Adds
pytest-rerunfailuresand enables a single automatic rerun with a short delay to mitigate transient failures. - Updates
test_modify_visibilityand live test DB setup to reduce cross-run state collisions (including scoping Asset lookup/creation by project).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
azure-pipelines-templates/run-tests.yml |
Adds pytest reruns, switches to per-build ephemeral project naming, and adds a cleanup step to delete the project after the job. |
tests/test_api.py |
Updates test_modify_visibility to use the ephemeral CI project as the primary toggle target and a separate project as a control. |
tests/base.py |
Adjusts Asset identity keys to include project, preventing cross-project collisions when codes are reused. |
tests/requirements.txt |
Adds pytest-rerunfailures to support the new --reruns options in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
julien-lang
left a comment
There was a problem hiding this comment.
Lookg pretty good! Thanks for the improvements.
Just a few comments.
julien-lang
left a comment
There was a problem hiding this comment.
Looks good. Just a last few minor comments.
julien-lang
left a comment
There was a problem hiding this comment.
Almost there, just 2 last comments
| os.environ['SG_SERVER_URL'], | ||
| os.environ['SG_SCRIPT_NAME'], | ||
| os.environ['SG_API_KEY'], | ||
| base_url=os.environ['SG_SERVER_URL'], |
There was a problem hiding this comment.
Sorry, no, not this one. base_url is a positional parameter, not a named parameter.
There was a problem hiding this comment.
Based on this line it looks like a named (but not optional) parameter.
python-api/shotgun_api3/shotgun.py
Line 500 in aca9d94
| scriptSource: inline | ||
| workingDirectory: $(Build.SourcesDirectory) | ||
| script: | | ||
| import os, shotgun_api3 |
There was a problem hiding this comment.
| import os, shotgun_api3 | |
| import os, shotgun_api3, sys |
Azure pipelines were occasionally failing for various reasons. Some were transient connection errors (timeouts), but sometimes it was live state issues or test run pollution: only certain runners were failing.
This PR creates ephemeral projects scoped to Build + OS + Python version, so concurrent jobs are fully isolated. These projects are deleted unconditionally on job end.
We also added retries on the tests for the transient error cases.
Also modified
test_modify_visibility, which was failing unpredictably (likely due to race conditions on using the same existing projects across runners). Now, it uses the ephemeral project for modifying the visibility, and uses the existing project only as a control group (so this one should never have it's visibility changed and not affect concurrent executions oftest_modify_visibility)Tested multiple times to ensure no more failures due to those identified issues
