-
Notifications
You must be signed in to change notification settings - Fork 123
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 docs for coverage.md and enable tests requirement for feature/bug… #1857
Conversation
Attached issue: https://pulp.plan.io/issues/7567 |
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.
LGTM
.travis/pre_before_install.sh
Outdated
# check if a test was added | ||
NEEDS_TEST=$(git diff --name-only $RANGE | grep -E 'feature|bugfix') | ||
CONTAINS_TEST=$(git diff --name-only $RANGE | grep -E '^test_') | ||
|
||
if [[ $(git log --format=medium --no-merges "$RANGE" | grep "\[notest\]") ]] | ||
then | ||
echo "[notest] is present - skipping the check for the test requirement" | ||
elif [ -n $NEEDS_TEST ] && ![ -n $CONTAINS_TEST ]; then | ||
echo "Every feature and bugfix should come with a test." | ||
exit 1 | ||
fi | ||
|
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 had a similar approach at pulp_file and it didn't work as I expected:
- Require tests for feature/bugfix pulp_file#426
- https://travis-ci.com/github/pulp/pulp_file/jobs/382763671#L283
Could you add a changelog entry .feature
to verify if it is working?
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 actually took the code from that PR, I will test with .feature entry
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.
the travis still does not work as expected, i need to look more into this.
bd7d1b4
to
fdacb2b
Compare
b631e70
to
63c9899
Compare
this is ready for review, to see whether script is correct please refer to this PR build #1865 |
@ipanova All of the documentation changes look good to me, and most of the Travis changes do -- but I am not quite fluent enough with bash to give a proper review of those parts. |
@ipanova++ |
…fix.
closes #7567
https://pulp.plan.io/issues/7567