TST/CI: Split Circle workflow #2386
Conversation
effigies
force-pushed
the
ci/workflow
branch
2 times, most recently
from
a80dbbd
to
7133d79
Compare
|
This is working. There's a definite slowdown due to saving and loading images, so perhaps builds should be moved back into the test jobs. Still, it looks like we can achieve concurrency across PRs. |
|
From the latest build: 17:58 to build images 12:38 -12:47 to attach to workspace and load images So for a cost of about 5 minutes per test job, we can skip a 30 minute bottleneck. Seems like a good trade-off. |
effigies
force-pushed
the
ci/workflow
branch
3 times, most recently
from
04eeaef
to
7e7dd79
Compare
.circleci/config.yml
Outdated
| - image: docker:17.10.0-ce-git | ||
| working_directory: /home/circleci/nipype | ||
| steps: | ||
| - setup_remote_docker |
There was a problem hiding this comment.
the remote docker might be useful when building, but it is not necessary for running. I may be wrong, but I think that remote docker does not allow mounting, and you need to do docker cp. The different jobs may be just using the local docker install. IMHO this line can be removed on the test jobs.
.circleci/config.yml
Outdated
| command: | | ||
| pip install --no-cache-dir codecov | ||
| - run: | ||
| name: Download test data |
There was a problem hiding this comment.
Maybe we can even split further and take this out as a different job?
.circleci/config.yml
Outdated
| environment: | ||
| WORKDIR: /home/circleci/work | ||
| command: | | ||
| mkdir -p "$WORKDIR" | ||
| chmod -R 777 "$WORKDIR" | ||
| bash /home/circleci/nipype/.circleci/tests.sh | ||
| bash /home/circleci/nipype/.circleci/pytests.sh | ||
| - store_artifacts: |
There was a problem hiding this comment.
I'd use store_test_results here, so we can have the test summary.
effigies
changed the title
|
@kaczmarj could you have a quick look? Thanks! |
|
There are some issues with codecov not pointing to the right files. I think I want to move the codecov to its own entry in Circle, which would let us simplify the script structure, too. I just haven't found much time to figure out where the coverage files are/should be. |
|
also we should ensure that some of the artifacts are being saved, such as the built docs and the coverage files. |
|
oh, one more: use set up the test summaries (https://circleci.com/docs/2.0/configuration-reference/#store_test_results) so that circle can show us number of tests and time in the |
|
Well, this branch is open to pushes from team members... Anyway, I can try to find some time later this week, but I think it's going to take a couple periods of sustained attention, so no definite time frame. |
|
Sorry for the delay. This looks great to me. One thing I would suggest is to reduce duplication in in For example: _modify_nipype_version: &modify_nipype_version
name: Modify Nipype version if necessary
command: |
if [ "$CIRCLE_TAG" != "" ]; then
sed -i -E "s/(__version__ = )'[A-Za-z0-9.-]+'/\1'$CIRCLE_TAG'/" nipype/info.py
fi
# ...
- run: *modify_nipype_version
# ...
- run: *modify_nipype_versionI can try to add this over the weekend. Otherwise this looks great! |
|
That is super handy to know. If you just want to ping this thread when
you're going to work on something, that'll keep me from doing the same
thing if I happen to have time to work on it at the same time. (I'll do the
same.)
…On Feb 9, 2018 17:27, "Jakub Kaczmarzyk" ***@***.***> wrote:
Sorry for the delay. This looks great to me. One thing I would suggest is
to reduce duplication in in .circleci/config.yml with yaml pointers.
Common procedures can be defined once and referenced multiple times.
For example:
_modify_nipype_version: &modify_nipype_version
name: Modify Nipype version if necessary
command: | if [ "$CIRCLE_TAG" != "" ]; then sed -i -E "s/(__version__ = )'[A-Za-z0-9.-]+'/\1'$CIRCLE_TAG'/" nipype/info.py fi# ...
- run: *modify_nipype_version# ...
- run: *modify_nipype_version
I can try to add this over the weekend. Otherwise this looks great!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2386 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFF8iw9AJ1a4WqcxX_tcP-qjCLNxB3cks5tTMZXgaJpZM4RljPx>
.
|
|
@effigies - I am working on this now |
Runtimes aggregated from https://circleci.com/workflow-run/721d8f15-dac6-4b7d-87c9-7dbeb1709f1a Two workflows require pre-run of other workflows.
effigies
force-pushed
the
ci/workflow
branch
from
8da6b37
to
e5be9ac
Compare
effigies
force-pushed
the
ci/workflow
branch
from
89f5160
to
a9c170a
Compare
effigies
force-pushed
the
ci/workflow
branch
from
4b42c65
to
575573e
Compare
effigies
force-pushed
the
ci/workflow
branch
from
575573e
to
8982b27
Compare
|
This is ready for review. Docs and coverage are in the artifacts. Codecov is pushing pull request numbers again, which should start showing up again once this hits master. And test summaries show up for the |
|
thanks @effigies - this looks good. |
Updating with todo list from comment thread:
Original text
This PR tracks an attempt to split out the testing process into a workflow of single-container jobs using the Circle 2.0 workflows. The goal is to reduce the amount of time containers are allocated but idle, and allow multiple PRs to do some simultaneous testing.
Follow up to #2202.
Comments and suggestions welcome. cc @kaczmarj