Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses regression failures in integration tests that were caused by timing issues during domain creation. The changes add polling mechanisms to wait for domains to become active before proceeding with tests, preventing 404 errors and timeouts.
Changes:
- Extended object storage cluster status validation to include "hidden" and "limited" states
- Enhanced
wait_for_conditionhelper to accept variable arguments for more flexible condition checking - Added
view_command_attributehelper utility for retrieving specific attributes from CLI commands - Implemented domain status polling in domain and event fixtures to ensure domains are active before test execution
- Added proper cleanup for test-created domains to prevent resource leakage
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/obj/test_object_storage.py | Added "hidden" and "limited" to valid cluster status values |
| tests/integration/helpers.py | Enhanced wait_for_condition to accept *args and added view_command_attribute helper |
| tests/integration/events/fixtures.py | Added domain status polling with wait_for_condition to events_create_domain fixture |
| tests/integration/domains/test_domain_records.py | Added cleanup for test-created domain and imported delete_target_id |
| tests/integration/domains/fixtures.py | Added domain status polling to all domain fixtures (master_domain, slave_domain, domain_and_record) |
Comments suppressed due to low confidence (1)
tests/integration/helpers.py:73
- The error message "SSH timeout expired" is misleading when this function is used for waiting on domain status. The function is now generic and accepts any condition via the condition parameter, so the error message should be more generic or customizable. Consider changing the message to something like "Timeout waiting for condition" or accepting a custom timeout message as a parameter.
if time.time() - start_time > timeout:
raise TimeoutError("SSH timeout expired")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Regression fixes for integration tests. Most of the failures were possibly related to long domain creation ([404] Not found, timeout)
Jira: https://track.akamai.com/jira/browse/TPT-4203
✔️ How to Test
Integration tests workflow run