Conversation
8838d81 to
50e8b35
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2319 +/- ##
============================================
- Coverage 80.93% 80.81% -0.12%
+ Complexity 1462 1459 -3
============================================
Files 139 139
Lines 6766 6766
Branches 728 728
============================================
- Hits 5476 5468 -8
- Misses 968 979 +11
+ Partials 322 319 -3 see 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
check for errors in tests ignore StatusDeadlineExceeded errors ____________________________________________________________________ Signed-off-by: berryware <231598+berryware@users.noreply.github.com>
50e8b35 to
de859a7
Compare
Nana-EC
left a comment
There was a problem hiding this comment.
Seems reasonable, but why the double error check?
| @@ -0,0 +1,58 @@ | |||
| import {Client, StatusDeadlineExceeded, StatusOK} from 'k6/net/grpc'; | |||
There was a problem hiding this comment.
| import {Client, StatusDeadlineExceeded, StatusOK} from 'k6/net/grpc'; | |
| // SPDX-License-Identifier: Apache-2.0 | |
| import {Client, StatusDeadlineExceeded, StatusOK} from 'k6/net/grpc'; |
There was a problem hiding this comment.
Is it possible to enable spotless for JS also? That would help with items as such, but will also introduce reliable formatting for the code.
| check(response, { | ||
| 'status is OK': (r) => r && r.status === StatusOK, | ||
| }); | ||
| if (response.status !== StatusOK) { |
There was a problem hiding this comment.
Q: why are we double checking the response?
It seems we expect StatusOK the first time but then not the second time but are curious of the failures the second time?
There was a problem hiding this comment.
This is one of the many k6 limitations we've hit. As we know, k6 has very poor support for gRPC. An unfortunate reality we have to deal with. There seems to be no issue with the BN itself.
@berryware you can consider to close #1963 also with this PR, as we noted there, this would be our best option, I see you have also updated the server status test as well.
| if [[ $? -ne 0 ]]; then | ||
| echo "Server status test FAILED." | ||
| # Optional: exit the script with a specific error code | ||
| exit 1 | ||
| else | ||
| echo "Server status test PASSED." | ||
| fi |
There was a problem hiding this comment.
Not sure if we should exit here. I did not add an exit first because we are still exploring this, but also, if we exit, we will not give a chance to other tests to run. Maybe a better approach would be to collect results and fail at the end, if any tests have failed, but run everything nonetheless. We will also have an overview of what failed/succeeded.
add in tests for serverStatusDetail
check for errors in tests
ignore StatusDeadlineExceeded errors