Skip to content
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

Check error code of nodetool command #3632

Merged
merged 1 commit into from
Apr 16, 2019
Merged

Conversation

therve
Copy link
Contributor

@therve therve commented Apr 16, 2019

This checks the error code of the nodetool command so that it logs an error if
it's not 0.

The command doesn't seem to use stderr, so checking err is not really useful,
and using the "Error:" prefix is not good enough as it's not present all the
time. This should fix it.

It also fixes the docker environment building so that it works with non local docker.

Copy link
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome ! Nice refactoring of the tests

@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #3632 into master will increase coverage by 8.45%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master    #3632      +/-   ##
==========================================
+ Coverage   86.02%   94.48%   +8.45%     
==========================================
  Files         741        7     -734     
  Lines       38996      145   -38851     
  Branches     4682       16    -4666     
==========================================
- Hits        33546      137   -33409     
+ Misses       4178        4    -4174     
+ Partials     1272        4    -1268

@therve therve merged commit 1b6d5c3 into master Apr 16, 2019
@ofek ofek deleted the therve/cassandra-nodetool-error branch June 4, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants