-
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 irc feature #279
Conversation
@yegor256 please, check this test out |
@zaqbez39me looks good, but |
@yegor256 check it out, please. I suppose, it is ready to be merged |
tests/metrics/test-irc.sh
Outdated
cd "${tmp}" | ||
mkdir -p "${LOCAL}/${temp}" | ||
touch "${LOCAL}/${temp}/file.java" | ||
"${LOCAL}/metrics/irc.sh" "${LOCAL}/${temp}/file.java" "${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
tests/metrics/test-irc.sh
Outdated
git config user.email '[email protected]' | ||
git config user.name 'Foo' | ||
file1="one.java" | ||
"${LOCAL}/metrics/irc.sh" "./${file1}" "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 also should fail
tests/metrics/test-irc.sh
Outdated
file1="one.java" | ||
touch "${file1}" | ||
"${LOCAL}/metrics/irc.sh" "./${file1}" "t0" | ||
grep "No commits yet in repo" "t0" # There are no commits in repo with given file |
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 the output is wrong, should be IRC 0
(or something similar)
@yegor256 please check it out. I changed the expected behaviour after your review. |
metrics/irc.sh
Outdated
|
||
output=$(realpath "$2") | ||
|
||
# TODO: #279 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 better remove this TODO
, since it will trigger the creation of a new issue in the backlog -- we don't want TWO issues to originate from this pull requests. just one is enough.
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
@yegor256 check it out, removed multiple TODOs referencing one PR |
@zaqbez39me thanks!@ |
Test for Impact Ratio by Commits (IRC) feature in #226