[ci] adding github action (closes #156)

miykael / nipype_tutorial

Learn Nipype with these tutorial notebooks - go here to see them online -->

https://miykael.github.io/nipype_tutorial

BSD 3-Clause "New" or "Revised" License

122 stars 69 forks source link

[ci] adding github action (closes #156) #159

Closed djarecka closed 4 years ago

djarecka commented 4 years ago

@mgxd, @effigies @satra - testing github actions, happy to get any feedback (especially if you have better idea how to deal with container management/caching)

and I'd really want to know how to enable it... when I was testing on my profile I haven't had to do anything extra, it just worked when i included .github/workflows, but here it doesn't (at least for this pr)

this is a ga dashboard from my fork (tests are failing right now): https://github.com/djarecka/nipype_tutorial/runs/957128296?check_suite_focus=true

EDIT general comments / for future consideration

I've decided to use multiple jobs in order to be able to run things in parallel
I didn't find THE way of passing the image to the next job, so I've decided to use uses: satackey/action-docker-layer-caching@v0.0.5 for now (didn't check if pull/push from dockerhub is not faster). I'm assuming at some point this will be better supported and we will want to change it

satra commented 4 years ago

this is still testing via circle. actions only start executing after they have at least been added to master. so if it works for you, merge this in and then the action will fire. after that PRs should do the right thing.

djarecka commented 4 years ago

a fun fact - the current version of the test_notebooks (2 years old?) where giving green light on CI regardless if tests were passing or failing... see for example run_tests section of circleCI: https://app.circleci.com/pipelines/github/miykael/nipype_tutorial/87/workflows/cbc2e7db-0dd5-4352-bd9c-fb767a6b7975/jobs/1352

mgxd commented 4 years ago

@djarecka it looks like the problem lies within the test script: https://github.com/miykael/nipype_tutorial/blob/ff7c4232aa3371254d444271e7058c7d620c66f6/test_notebooks.py#L94-L96

since the return value of the system call is not checked, any command would pass.

djarecka commented 4 years ago

@mgxd - yes, I noticed... Will fix this later, and than would have to review the notebooks that are failing, don't even want to know when they started failing....

djarecka commented 4 years ago

actually, I might just merge this first to have at least working CI structure (i will give up on finding out why CircleCI doesn't want to build the image for me)

djarecka commented 4 years ago

@satra - you were right, it works now! :) in my fork i didn't need to merge to master, but perhaps it makes sense to treat owners of the repo differently