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.

Sign up for GitHub

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

Jump to bottom

Add possibility to trigger specific module's tests in CI on Pull Requests via issue comments #1408

Merged
merged 20 commits into from
Oct 4, 2016
Merged

Add possibility to trigger specific module's tests in CI on Pull Requests via issue comments #1408

merged 20 commits into from
Oct 4, 2016

Conversation

megies
Copy link
Member

@megies megies commented Sep 26, 2016

For PRs that make changes on network modules, I think it would be good to have some possibility to trigger some specific network module's tests (alongside the default non-network modules tests) during CI. We could e.g. check the commit message for a magic regex, something like 'TEST:clients.fdsn'..

@megies megies added the testing issues generally related to our testing setup / infrastructure label May 17, 2016
@megies megies changed the title Add possibility to trigger specific module's tests via commit message in PR Add possibility to trigger specific module's tests in CI via commit message in PR May 17, 2016
@megies megies mentioned this pull request May 17, 2016
Closed
@krischer
Copy link
Member

That would indeed be very useful. Maybe we could also parse the corresponding github comments (if any) - then we could trigger it on other people's code.

@megies
Copy link
Member Author

megies commented May 23, 2016 •
edited
Loading

Yeah, that would be even better.. 👍

@megies megies added the CI issue generally related to continuous integration label May 23, 2016
@megies
Copy link
Member Author

megies commented Sep 26, 2016 •
edited
Loading

This should enable requesting specific modules' tests via comments.. e.g. using:

+TESTS:clients.fdsn,clients.iris

todo:

  • changelog documented in CONTRIBUTING.md, and effective immediately, so doesn't make much sense to file it in changelog under some specific version number
  • maybe some testing (see https://github.com/obspy/obspy_github_api/blob/0.3.0/obspy_github_api/tests/test_obspy_github_api.py
  • document it somewhere (documented in CONTRIBUTING.md under "PRs")
  • use also in CI
    • Travis
    • Appveyor (didn't do right now, not sure how to do it similar to Travis with Windows syntax)
    • docker-testbot (would make the docker scripts too complicated, do not want to add such a lot more complexity to it)
  • need to disable in appveyor "branch" (when %APPVEYOR_PULL_REQUEST_NUMBER% is not set)
  • probably need to adapt docs buildbot due to the helper scripts moved from misc folder to actual obspy API under core.util (started a new issue with Refactor CI scripts to use obspy/obspy_github_api #1534 to track this, currently docs-buildbot stuff is self-contained but should be refactored to use new core.util.github_api helper module) tinkering with github API for obspy specific use cases has been moved to separate obspy/obspy_github_api module
  • need to add encrypted env variable OBSPY_COMMIT_STATUS_TOKEN for github API (added it in travis/appveyor project settings online)
  • parse all +TESTS:... regex strings, instead of only the last one

@megies megies added this to the 1.1.0 milestone Sep 26, 2016
@megies megies force-pushed the test_specific_modules branch 2 times, most recently from 66563cd to faa5d71 Compare September 27, 2016 07:32
@megies megies changed the title Add possibility to trigger specific module's tests in CI via commit message in PR Add possibility to trigger specific module's tests in CI on Pull Requests via issue comments Sep 27, 2016
@megies megies mentioned this pull request Sep 27, 2016
Closed
@megies
Copy link
Member Author

megies commented Sep 27, 2016 •
edited
Loading

Works in principle but I'll put it on hold.. no time to work on refactoring this cleanly in misc for all the CI's.

@megies megies force-pushed the test_specific_modules branch 5 times, most recently from afacbfc to 956bcc5 Compare September 28, 2016 16:20
@megies
Copy link
Member Author

megies commented Sep 30, 2016

Only thing missing is also do the check for requested additional tests in appveyor, too. Not so good with the windows syntax, so leaving it out for now. In any case Travis will test any additionally requested modules (or ALL modules, if requested).

@megies megies force-pushed the test_specific_modules branch 3 times, most recently from 3c741ff to 6a684cb Compare September 30, 2016 12:31
@krischer
Copy link
Member

krischer commented Oct 3, 2016

Some of the failing tests should work if you rebase on the lastest master.

@megies
docker tests: also check github PR comments for additional modules to
ef19faf 
test
@megies
move CONTRIBUTING.md back to top-level, should not be visivle and not…
ea3bcc1 
… hidden in a

special folder `.github`, IMHO
@megies
move github api helper routines back to misc
1821977
@megies
misc: github API helpers: refactor to use github3.py module
a065c82
@megies
refactoring
69f0815
@megies
revert changes to run_tests() and obspy-runtests, adding specific module
b1b9dfb 
tests will have to be handled differently, in CI scripts

Revert "docker tests: also check github PR comments for additional modules to"

This reverts commit e479742.

Revert "CI: parse issue comments for regex to run tests for specific modules"

This reverts commit faa5d71.

Revert "run_tests: add option to check for requests of specific (network) module"

This reverts commit 90eaa28.
@megies
remove misc/github, moved to obspy/obspy_github_api repo
7d6af81
@megies
refactor docs buildbot scripts to use obspy_github_api
0c380c2
@megies
Travis: use obspy_github_api to check if additional modules should be
a58c2b9 
tested for a PR
@megies
travis: fix variable substitution in runtests call
023af2a
@megies
adapt to obspy_github_api 0.3.0
c680d36
@megies
Copy link
Member Author

megies commented Oct 3, 2016

Rebased on current master (hope I got those conflicts right..)

@megies
Copy link
Member Author

megies commented Oct 3, 2016 •
edited
Loading

Todo:

  • run network tests in at least one docker container per day on maintenance_1.0.x and master and submit commit status

@megies
Copy link
Member Author

megies commented Oct 4, 2016

run network tests in at least one docker container per day on maintenance_1.0.x and master and submit commit status

Hmm.. thinking about it, the other option would be to set Travis to test all modules for pushes (as opposed to PRs) to master/maintenance. That wouldn't impede speed of feedback on PRs commit status and would move those tests more to the core of our CI setup.

btw. @krischer, there's a test fail in fdsn mass downloader on old scipy (https://travis-ci.org/obspy/obspy/jobs/164580925), not sure if it's worth to fix..

@krischer
Copy link
Member

krischer commented Oct 4, 2016

Hmm.. thinking about it, the other option would be to set Travis to test all modules for pushes (as opposed to PRs) to master/maintenance. That wouldn't impede speed of feedback on PRs commit status and would move those tests more to the core of our CI setup.

Sounds good. But I guess then we really need to make our networks tests more stable.

btw. @krischer, there's a test fail in fdsn mass downloader on old scipy (https://travis-ci.org/obspy/obspy/jobs/164580925), not sure if it's worth to fix..

Probably not worth to fix but the tests should be skipped for old scipy versions.

@megies
Copy link
Member Author

megies commented Oct 4, 2016

But I guess then we really need to make our networks tests more stable.

Doesn't matter much actually.. any commit status with whatever context that sets a status of "error" or "failed" will make our build show up as broken..

@megies
CONTRIBUTING.md: document +DOCS and +TESTS:... magic for PRs
bc56429 
..and also purge outdated comment on base branch switching
@megies
Copy link
Member Author

megies commented Oct 4, 2016 •
edited
Loading

Good to merge, I think. (stray Travis fail is due to some fdsn mass downloader issues with old scipy)

Short summary of what this now actually does:

  • on Travis, for PR builds: install and use obspy/obspy_github_api to check whether any specific modules should be tested for the PR (using e.g. +TESTS:clients.fdsn,clients.iris) and test them.. ;-)
  • CONTRIBUTING.md is moved back out of the .github folder into bright daylight and a note on +DOCS and +TESTS:... magic is added for documentation
  • tinkering with github API is moved from several places in misc/... to a pip-installable module obspy/obspy_github_api

@megies megies mentioned this pull request Oct 4, 2016
Merged
@krischer
Copy link
Member

krischer commented Oct 4, 2016

Looks good to me :-)

@krischer krischer closed this Oct 4, 2016
@krischer krischer reopened this Oct 4, 2016
@krischer krischer merged commit dc38b1e into master Oct 4, 2016
@krischer krischer deleted the test_specific_modules branch October 4, 2016 14:09
@megies megies mentioned this pull request Dec 2, 2016
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CI issue generally related to continuous integration testing issues generally related to our testing setup / infrastructure
Projects
None yet
Milestone
1.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@megies @krischer