-
Notifications
You must be signed in to change notification settings - Fork 18
Fix checkpayment tests #847
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.
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #847 +/- ##
==========================================
- Coverage 64.85% 64.82% -0.03%
==========================================
Files 88 88
Lines 8163 8159 -4
Branches 734 734
==========================================
- Hits 5294 5289 -5
- Misses 2639 2641 +2
+ Partials 230 229 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0b348ad to
72890e7
Compare
…ble use another CCN URL to try to connect it." Fix: Refactor API failover to prevent test timeouts
cd63201 to
ff48e9f
Compare
00fb772 to
35b842b
Compare
This reverts commit d58b1f3.
… when mounting the base image.
| await execution.start() | ||
| await execution.stop() | ||
| await asyncio.wait_for(execution.start(), timeout=300) | ||
| await asyncio.wait_for(execution.stop(), timeout=300) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why so long?
| mkdir -p /opt/aleph/libs | ||
| # Fixing this protobuf dependency version to avoid getting CI errors as version 5.29.0 have this compilation issue. | ||
| pip3 install --target /opt/aleph/libs 'aleph-sdk-python==2.0.5' 'aleph-message~=1.0.5' 'fastapi~=0.120.1' 'protobuf==6.33.0' | ||
| pip3 install --target /opt/aleph/libs 'aleph-sdk-python==2.0.0' 'aleph-message~=1.0.5' 'fastapi~=0.121.0' 'protobuf==5.28.3' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be changed by this PR. Revert?
| return 500 * len(executions) | ||
|
|
||
| mocker.patch("aleph.vm.orchestrator.tasks.compute_required_flow", compute_required_flow) | ||
| mocker.patch("aleph.vm.orchestrator.tasks.compute_required_flow", new=compute_required_flow) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this change is pertinent, it's more confusing than anything. I had to look up the doc to understand that new= was the second argument and that therefore this change changes nothing.
| mocker.patch( | ||
| "aleph.vm.orchestrator.tasks.get_community_wallet_address", | ||
| new=mocker.AsyncMock(return_value=mock_community_wallet_address), | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference in behavior with
mocker.patch("aleph.vm.orchestrator.tasks.get_community_wallet_address", return_value=mock_community_wallet_address)
? I don't think there's one.
| return {"result": False, "reason": str(e)} | ||
| except Exception as e: | ||
| logger.error(f"Unexpected error in URL check: {e}", exc_info=True) | ||
| return {"result": False, "reason": str(e)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is reimplementing what Andres did in another way right? Why?
tests was hanging because async mocks was not correctly configure