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

Add diffing support to bash tests #531

Closed
MichaelDimmitt opened this issue Aug 17, 2021 · 8 comments · Fixed by #539
Closed

Add diffing support to bash tests #531

MichaelDimmitt opened this issue Aug 17, 2021 · 8 comments · Fixed by #539

Comments

@MichaelDimmitt
Copy link

MichaelDimmitt commented Aug 17, 2021

Diffing is easy to attain in bats
If you add printDiff to every function.
It will only show the output if the test fails.

curious if @IsaacG has thoughts.

by changing

@test "a name given" {
  [[ $BATS_RUN_SKIPPED == true  ]] || skip
  run two_fer Alice
  [[ $status -eq 0 ]]
  [[ $output == "One for Alice, one for me." ]]
}
printDiff(){
  echo output;
  echo "$output";
  echo ;
  echo expected;
  echo "$expected;
}

@test "a name given" {
  [[ $BATS_RUN_SKIPPED == true  ]] || skip
  run two_fer Alice

  expected=$(cat <<-END
One for Alice, one for me.
END
  )

  printDiff
  [[ $status -eq 0 ]]
  [[ "$output" ==  "$expected" ]]
}

This actually just saved me on a project.
And I am sourcing the scripts like libs as is proposed in the pending pr
#373

@MichaelDimmitt MichaelDimmitt changed the title add diffing support to bash tests Add diffing support to bash tests Aug 17, 2021
@IsaacG
Copy link
Member

IsaacG commented Aug 17, 2021

My name was mentioned?

Drop the trailing semicolons. Use snake case naming consistently, including function names. Prefer printf over echo. Replace your $(cat <<<END ... foo) with "foo". Prefer (( over [[ for numeric tests (-eq 0). Only print diff when there is a diff.

@IsaacG
Copy link
Member

IsaacG commented Aug 17, 2021

If you're going to show diffs, you may want to actually show the diff, not just the want/expect. diff is usually installed on most boxes that support bash. Maybe diff -u0 <("$want") <("$got")

@MichaelDimmitt
Copy link
Author

Sorry about the mention, you always give great feedback
thanks for the insight I will try it out diff -u0 <("$want") <("$got")

@IsaacG
Copy link
Member

IsaacG commented Aug 19, 2021

@MichaelDimmitt Feel free to mention me any time you want someone pedantic to nitpick your shell code :)

@MichaelDimmitt
Copy link
Author

Went with cmp for readibility purposes

also started a discussion on bats-core 🎉 bats-core/bats-core#490

printDiff(){
  echo ;
  echo output;
  echo "$output";
  echo ;
  echo expected;
  echo "$expected";
  echo ;
  cmp <(echo "$output") <(echo "$expected") | cut -c 23-;
  echo ;
}

@test "a failure on line 1" {
  run bash two_fer.sh Alice
  expected=$(cat <<-END
test should fail
this is some text
END
  )
  output=$(cat <<-END
test should pass
this is some text
END
  )
  printDiff
  [[ "$output" == "$expected" ]]
}
@test "a failure on line 2" {
  run bash two_fer.sh Alice
  expected=$(cat <<-END
this is some text
test should fail
END
  )
  output=$(cat <<-END
this is some text
test should pass
END
  )
  printDiff
  [[ "$output" == "$expected" ]]
}
@test "a bad diff occurs if expected is 2 lines but output is 1 line" {
  run bash two_fer.sh Alice
  expected=$(cat <<-END
this is some text
test should fail
END
  )
  output=$(cat <<-END
this is some text
END
  )
  printDiff
  [[ "$output" == "$expected" ]]
}

Screen Shot 2021-08-22 at 11 11 45 AM

@IsaacG
Copy link
Member

IsaacG commented Aug 22, 2021

On a successful test, will this echo the output two times?

@MichaelDimmitt
Copy link
Author

MichaelDimmitt commented Aug 24, 2021

If the test passes the terminal does not output the results of echo from printDiff

The output only shows when the bash test fails.

Screen Shot 2021-08-26 at 8 59 55 PM

@glennj
Copy link
Contributor

glennj commented Sep 3, 2021

I'm coming around to this. With the v3 release of exercism, showing students that [[ $output == $expected ]] fails is just insufficient information.

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 a pull request may close this issue.

3 participants