-
Notifications
You must be signed in to change notification settings - Fork 41
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
Implemented test for ir feature #259
Conversation
@yegor256 please, check this test out |
@yegor256 please, check this test. The pipeline fails because the test is not executable for some reasons. It offers to run |
@yegor256 fixed the test disabling method |
@zaqbez39me I suggest you wait until the build is clean and then ping me |
@zaqbez39me yes, indeed, try to run |
@yegor256 check it out, please. I suppose, it is ready to be merged |
tests/metrics/test-ir.sh
Outdated
tmp=$(mktemp -d /tmp/XXXX) | ||
cd "${tmp}" | ||
mkdir -p "${LOCAL}/${temp}" | ||
"${LOCAL}/metrics/ir.sh" ./ "${LOCAL}/${temp}/stdout" |
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.
@zaqbez39me should fail here
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.
@yegor256 you want me to expect the ir.sh
failing instead of printing anything here?
tests/metrics/test-ir.sh
Outdated
git init --quiet . | ||
git config user.email '[email protected]' | ||
git config user.name 'Foo' | ||
"${LOCAL}/metrics/ir.sh" ./ "t0" |
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.
@zaqbez39me should fail here
tests/metrics/test-ir.sh
Outdated
git add "${file2}" | ||
git commit --quiet -m "second file" | ||
"${LOCAL}/metrics/ir.sh" ./ "t2" | ||
grep "No files in given repo " "t0" # There are no files in repo |
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.
@yegor256 please check it out. I changed the expected behaviour after your review. |
tests/metrics/test-ir.sh
Outdated
set -e | ||
set -o pipefail | ||
|
||
# TODO: #259 ENABLE THIS TESTS RIGHT AFTER IMPLEMENTING ir.sh VIA REMOVING `exit 0` |
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.
@zaqbez39me if you use TODO
twice -- two issues will be created in the backlog by the robot. We don't want this. Better keep one TODO
, where explain what needs to be done in both files
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.
I see. Thanks, will follow it
@zaqbez39me looks good, just one small comment |
|
metrics/ir.sh
Outdated
|
||
output=$(realpath "$2") | ||
|
||
# TODO: #259 REPLACE WITH ACTUAL METRIC CODE |
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.
@zaqbez39me remove this TODO
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.
@yegor256 removed it
@zaqbez39me thanks! |
Test for Impact Ratio (IR) feature in #226