-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(core): add complex test for routing list #2760
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2760 +/- ##
==========================================
+ Coverage 87.12% 88.03% +0.90%
==========================================
Files 138 138
Lines 9377 9377
==========================================
+ Hits 8170 8255 +85
+ Misses 1207 1122 -85
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
actually i belive #2500 is fixed due to https://github.com/jina-ai/jina/pull/2750/files#diff-d9002011181954eeec0b9b94a9d08f7bc8028338275a35bea7823aa07d08041dL33 |
d65b657
to
3605823
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.
can you benchmark these two test if slow mark it as pytest.mark.slow
?
I checked its 3 seconds. Lets not mark it as slow as its rather nice to execute locally (no docker etc involved) |
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.
LGTM!
This adds a test to verify that #2500 got fixed as a side effect of dynamic routing (#2551).
Not sure we need this test actually as part of the test suite, but I was writing it anyway to verify the behaviour and I think there are no other tests doing this.
┆Issue is synchronized with this Asana task by Unito