BE-704 | Fix e2e tests #636
Conversation
WalkthroughThis pull request updates configuration and constants for integration testing. The workflow dispatch for integration tests now defaults to and supports an “integrators” environment instead of “stage.” Additionally, the Coingecko API URL is updated to a new endpoint. New service mappings for the integrators environment are introduced in the test configuration and SQS service files, including the addition of a constant for the integrators endpoint and an API token header in SQS requests. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHubActions as GitHub Actions
participant IntegrationJob as Integration Test Job
participant SQS as SQSService
Developer->>GitHubActions: Trigger workflow_dispatch (env: integrators)
GitHubActions->>IntegrationJob: Run scheduled integration test (env: integrators)
IntegrationJob->>SQS: Initialize SQSService with API key
SQS-->>IntegrationJob: Return response (with "x-api-token" header)
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/conftest.py (2)
56-56: Fix typo in constant name.There's a typo in the constant name
INEGRATORS_INPUT_NAME- it should beINTEGRATORS_INPUT_NAME(missing the 'T').-INEGRATORS_INPUT_NAME = "integrators" +INTEGRATORS_INPUT_NAME = "integrators"
64-64: Update variable reference after fixing typo.Update the reference in the INPUT_MAP to use the corrected constant name.
- INEGRATORS_INPUT_NAME: SQS_INTEGRATORS, + INTEGRATORS_INPUT_NAME: SQS_INTEGRATORS,🧰 Tools
🪛 Ruff (0.8.2)
64-64:
SQS_INTEGRATORSmay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/integration-test.yml(2 hunks)tests/coingecko_service.py(2 hunks)tests/conftest.py(2 hunks)tests/sqs_service.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/conftest.py
49-49: SQSService may be undefined, or defined from star imports
(F405)
49-49: SQS_INTEGRATORS may be undefined, or defined from star imports
(F405)
50-50: SQSService may be undefined, or defined from star imports
(F405)
50-50: SQS_LOCAL may be undefined, or defined from star imports
(F405)
51-51: CoingeckoService may be undefined, or defined from star imports
(F405)
62-62: SQS_STAGE may be undefined, or defined from star imports
(F405)
63-63: SQS_PROD may be undefined, or defined from star imports
(F405)
64-64: SQS_INTEGRATORS may be undefined, or defined from star imports
(F405)
97-97: SQS_INTEGRATORS may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Summary
🔇 Additional comments (7)
tests/coingecko_service.py (1)
4-4: Good API endpoint update.Updating the Coingecko API URL from the Osmosis-specific endpoint to the official Coingecko API endpoint. This should help fix the e2e tests as mentioned in the PR objectives.
tests/sqs_service.py (2)
5-5: Good addition of integrators environment endpoint.Adding the new SQS_INTEGRATORS constant for the integrators environment, which aligns with the PR objectives of fixing e2e tests.
35-35:❓ Verification inconclusive
Verify if both API headers are required.
The code now sends both
x-api-keyandx-api-tokenheaders with the same value. Is this redundancy intentional and required by the integrators environment?
🏁 Script executed:
#!/bin/bash # Check if both headers are used in the codebase echo "Checking for x-api-token usage in the codebase" rg -A 3 -B 3 "x-api-token" --type pyLength of output: 458
Header Redundancy: Confirm Intentional Dual Headers
The grep output confirms that both the
x-api-keyandx-api-tokenheaders are set with the same value intests/sqs_service.py. Please verify with the integrators whether this redundancy is intentional—perhaps to support backward compatibility or meet specific requirements—and update the implementation if one header is unnecessary.tests/conftest.py (2)
49-49: Good addition of integrators service.Adding the SQS service for the integrators environment, which aligns with the changes in other files.
🧰 Tools
🪛 Ruff (0.8.2)
49-49:
SQSServicemay be undefined, or defined from star imports(F405)
49-49:
SQS_INTEGRATORSmay be undefined, or defined from star imports(F405)
97-97: Good service mapping addition.Adding the service mapping for the integrators environment in the SERVICE_MAP dictionary.
🧰 Tools
🪛 Ruff (0.8.2)
97-97:
SQS_INTEGRATORSmay be undefined, or defined from star imports(F405)
.github/workflows/integration-test.yml (2)
12-14: Good environment configuration update.Adding the integrators environment option and making it the default for the workflow dispatch is consistent with the other changes in this PR. This should help fix the e2e tests as mentioned in the PR objectives.
54-54: Good scheduled test environment update.Updating the scheduled test to use the integrators environment is consistent with the other changes in this PR.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/integration-test.yml (3)
11-14: Environment Options and Default Value Updated.
The update correctly addsintegratorsas one of the environment options and sets it as the default value. However, note that theprodoption still appears in the list—please confirm that allowing selection of "prod" is intentional for these tests, as it might lead to unintended behavior if chosen.
47-48: Trailing Whitespace and Comment Clarity.
Static analysis flagged trailing spaces on line 48. Please remove any extra whitespace to avoid YAML lint errors. Additionally, the commented-outCOINGECKO_API_KEYline appears in this block; if it’s no longer needed or is superseded by other configuration, consider removing it for clarity.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 48-48: trailing spaces
(trailing-spaces)
75-76: Redundant Commented API Key in Scheduled Job.
Similar to the manual test job, the scheduled job includes a commented-outCOINGECKO_API_KEYline. Please verify if leaving these commented-out lines is deliberate; if they no longer serve a purpose, removing them can help maintain a cleaner workflow file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration-test.yml(3 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/integration-test.yml
[error] 48-48: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (1)
.github/workflows/integration-test.yml (1)
53-54: Matrix Environment Configuration Update.
The matrix configuration for thescheduled_integration_testjob now uses[integrators], aligning with the updated default environment. This change looks correct and ensures consistency across the workflow.
|
Summary by CodeRabbit
New Features
Tests