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

dshbak: fix handling of empty input lines #133

Merged
merged 2 commits into from
Aug 13, 2021
Merged

Conversation

grondo
Copy link
Member

@grondo grondo commented Aug 11, 2021

As described in #132 by @negregg, the fix for #70 inadvertently broke empty input in dshbak.

This PR adds a test to t5000-dshbak.sh to ensure empty input is handled by the script and then fixes the original fix.

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

LGTM

Problem: The dshbak tests do not test for empty input to the script,
so a regression that broke the script on empty lines/input was able
to get past make check.

Add a test to ensure that empty input works with dshbak.
Problem: The fix for hostnames of `0` in dshbak, i.e. the
following commit

 commit dee6adf
 Author: Mark A. Grondona <mgrondona@llnl.gov>
 Date:   Thu Jun 25 13:37:51 2015 -0700

    dshbak: Handle hostname of "0"

    @garlick found that dshbak can't handle the smallest of numbers.
    This is because dshbak skips lines where $tag resolves to false.
    But in perl, 0 is also false.

    What we really want to check for is an "empty" tag, so fix up the
    test to reflect that.

    Resolves chaos#70.

breaks handling of empty input to dshbak, since now the variable
"$tag" can be used when it is uninitialized. The proper fix for
issue chaos#70 was to check if the variable was either uninitialized or
an empty string.

Fixes chaos#132
@codecov-commenter
Copy link

Codecov Report

Merging #133 (b32d9d3) into master (d2746ff) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #133   +/-   ##
=======================================
  Coverage   63.81%   63.81%           
=======================================
  Files          27       27           
  Lines        5585     5585           
=======================================
  Hits         3564     3564           
  Misses       2021     2021           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2746ff...b32d9d3. Read the comment docs.

@mergify mergify bot merged commit 4f4927a into chaos:master Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants