ci: updated #46
ci: updated #46
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Equinix Fabric v4 test runner workflow and improves test reliability by adding PR-triggered CI, clarifying test ownership through Slack notifications, updating test user references, and increasing provisioning wait times.
Changes:
- CI workflow now triggers on pull requests to main branch and adds responsibility tracking to Slack notifications
- Test user reference updated from
PANTHERS_FNVtoPANTHERS_FCRacross multiple test classes - Increased BGP provisioning wait time from 3 to 10 seconds to improve test reliability
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/fabricv4_test_runner.yml | Added PR trigger, updated secret syntax to single quotes, and added responsibility mention to Slack notifications |
| equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/CloudRoutersApiTest.java | Changed test user from PANTHERS_FNV to PANTHERS_FCR |
| equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/NetworksApiTest.java | Changed test user from PANTHERS_FNV to PANTHERS_FCR |
| equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/RoutingProtocolsApiTest.java | Increased sleep interval from 3000ms to 10000ms for BGP provisioning wait |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| try { | ||
| Thread.sleep(3000); | ||
| Thread.sleep(10000); |
There was a problem hiding this comment.
Using Thread.sleep() for polling provisioning status is inefficient and can lead to unnecessarily long test execution times. Consider implementing an exponential backoff strategy or a more sophisticated polling mechanism with a configurable timeout and shorter intervals. This would allow the test to proceed as soon as the resource is provisioned rather than always waiting the full 10 seconds.
| *Test Runs:* ${{ needs.test.outputs.tests_run }} | ||
| *Results:* ${{ steps.set-output-passed.outputs.total_passed }} Passed, ${{ steps.set-output-failed.outputs.total_failed }} Failed | ||
| *Triggered by:* <@${{ github.actor }}> | ||
| *Responsibility:* <@U04AM0XBJMA> |
There was a problem hiding this comment.
The hardcoded Slack user ID makes the workflow less maintainable. Consider using a workflow input variable or repository secret for the responsible person's Slack ID. This would make it easier to update the responsible person without modifying the workflow file directly.
| *Responsibility:* <@U04AM0XBJMA> | |
| *Responsibility:* <@${{ secrets.SLACK_RESPONSIBLE_USER_ID }}> |
There was a problem hiding this comment.
@tutkat I added this secret to the repo settings, so we can take on this change in a subsequent PR.
This pull request updates the Equinix Fabric v4 test runner workflow and several test classes to improve test reliability and clarify test responsibility. The main changes include triggering the workflow on pull requests, updating test user references, increasing wait times for provisioning, and adding responsibility information to workflow notifications.
CI/CD Workflow Improvements:
.github/workflows/fabricv4_test_runner.ymlworkflow is now triggered on pull requests to themainbranch, ensuring tests run automatically for new changes.Test Reliability and Maintenance:
PANTHERS_FNVtoPANTHERS_FCRin bothCloudRoutersApiTestandNetworksApiTestto ensure tests use the correct user context. (equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/CloudRoutersApiTest.java[1]equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/NetworksApiTest.java[2]RoutingProtocolsApiTest.waitForBGPTypeIsProvisionedfrom 3 seconds to 10 seconds to improve reliability when waiting for resource provisioning. (equinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/RoutingProtocolsApiTest.javaequinix-openapi-fabric-tests/src/test/java/com/equinix/openapi/fabric/tests/RoutingProtocolsApiTest.javaL133-R133)