-
Notifications
You must be signed in to change notification settings - Fork 146
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
JDC Fallback test #1486
JDC Fallback test #1486
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1486 +/- ##
=======================================
Coverage 18.97% 18.97%
=======================================
Files 135 135
Lines 9466 9468 +2
=======================================
+ Hits 1796 1797 +1
- Misses 7670 7671 +1 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5b7bf18
to
54e53b5
Compare
54e53b5
to
a5d299a
Compare
b3d0584
to
e60addf
Compare
Super nice to see CI passing here! This is ready for review btw. |
e60addf
to
fee700d
Compare
This is ready for review |
4d572c0
to
e51a71d
Compare
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.
73bb302
to
b12e62c
Compare
8aa8719
to
cfb256f
Compare
Have we officially decided to add a file for every test? I see that the tracker issue here is still opened without a final conclusion. Maybe it's worth discussing this during the next PR Review Club call. |
Yea in one of the last calls we agreed that at least tests with a miner should be in a separate file. |
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 is this file being added on this commit?
I understand the idea of separating tests with CPU miners, but:
- this commit is titled "Allow
start_jdc
to accept multiple pool addresses", which doesn't have much to do with this specific change - we create this file but the test remains duplicated on
translator_integration.rs
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.
oops that was not intended
5cbb3a7
to
7c5febb
Compare
b4352ba
to
3d44c91
Compare
3d44c91
to
eb27cbe
Compare
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.
ACK but looks like CI is broken
.. this timeout is used before switching pool's
.. otherwise it can go into an endless loop(which surprisingly did not happen until now..)
Add a test to validate the behavior of JDC upon receiving a SubmitShareError message from the connected Pool. JDC is expected to switch to a different pool/jds pair if specfied in the config, otherwise it should do solo mining. + Remove jdc-upstream-change MG test
eb27cbe
to
03011f2
Compare
blocked by #1485
superseded #1343
Closes #1207