Split `ci.yml` into separate actions #280

TimothyWillard · 2024-08-02T18:30:14Z

Split the "unit-tests" action into multiple actions, currently one for each package contained within the flepiMoP repo. Also updated checkout from v3 to v4 to address node16 deprecation warnings and swapped ubuntu 20.04 for ubuntu latest. Changed the gempyor CI to not print stdout and exit on first failure.

See GH-278.

Split the "unit-tests" action into multiple actions, currently one for each package contained within the `flepiMoP` repo. Also updated checkout from v3 to v4 to address node16 deprecation warnings and swapped ubuntu 20.04 for ubuntu latest. Changed the gempyor ci to not print stdout and exit on first failure.

TimothyWillard · 2024-08-02T18:39:15Z

That did not do quite what I expected. Let me make a few small edits before this is ready for review.

Typo in `setwd` call causes error about not being able to change to directory that doesn't exist.

Set the shell to bash so the `source` function is available.

Add the same path related limits from the on push to on pull_requests as well.

TimothyWillard · 2024-08-02T19:01:02Z

These changes are now ready for review.

pearsonca

Seems conceptually fine, but I'm not expert enough on GH actions syntax.

shauntruelove

these look good. one minor thing to fix is to remove breaking-improvements branch as i believe that is gone. Are there other branches that should be included?

.github/workflows/flepicommon-ci.yml

.github/workflows/gempyor-ci.yml

Removed the `breaking-improvements` branch from special consideration in GitHub actions.

TimothyWillard · 2024-08-06T21:11:27Z

Ah, I did not realize that we had sunsetted the breaking-improvements branch, but that makes sense. I kept the dev branch, but I also don't see that as a branch on GitHub so should I remove that as well?

jcblemai · 2024-08-12T15:45:45Z

Look good to me, thanks. As mentionned, perhaps the verbosity and all tests with failure were useful as I know from the commit log we have folks using ci as way to test code. But I agree it's better to not have them in Github action, and we can add it back.

the splitting is very welcome.

Minor edits to run the gempyor tests with python 3.10 and 3.11. Remove usage of custom docker container.

Very minor edit to `__init__.py` to trigger the `gempyor` CI GitHub action.

Need to install `test` extra installs to get pytest instead of `dev`.

TimothyWillard · 2024-10-18T12:47:18Z

This PR is ready for review again. This PR now:

Splits the CI action into separate actions for gempyor, flepicommon and inference. Notably, this excludes flepiconfig which is also not included in the current CI action and does not have a passing test suite (see [Bug]: flepiconfig Test Suite Currently Does Not Pass #350).
This runs the GitHub actions against explicit R and python versions, which clarifies the environments that we support. For now R 4.3.3 and Python 3.10 and 3.11.
Uses caching to speed up the R package environment setup. These caches are reset weekly, but this can be adjusted if this is painful on a Monday. It doesn't do the same for python since that is quick to do, but could also be done in the future if we see slow downs in setup speed.

Some open questions:

I kept the dev branch, but I also don't see that as a branch on GitHub so should I remove that as well?
I suspect there will be some repo settings changes required to accommodate this (see how it is expecting an action called "unit-tests" to run below). Who should I work with to accomplish this?

Steps after this PR:

Investigate switching the environment setup to use conda instead of doing it manually after Generic HPC Install Script #329.
Incorporate flepiconfig into this set of GitHub actions.
Ensure that the workflow chaining works as expected, see: Split ci.yml into separate actions #280 (comment).

pearsonca

Looks good; one weird space diff, but that's likely test cruft?

pearsonca · 2024-10-18T14:34:03Z

flepimop/R_packages/inference/R/inference_to_forecast.R

@@ -28,6 +28,7 @@ cum_death_forecast <- function (sim_data,

 }

+


Looks good; one weird space diff, but that's likely test cruft?

Yeah, exactly. Just wanted to demonstrate what it would look like, but not needed and reverted.

Remove the changes used to trigger the workflows as an example. Not needed for the PR.

TimothyWillard · 2024-10-18T15:16:05Z

Removed the whitespace changes, but I it would be helpful to get answers to the above open questions before merging.

out if date

pearsonca

Still approve, but also still some weird whitespace diffs?

jcblemai · 2024-10-18T15:40:30Z

Still approve, but also still some weird whitespace diffs?

where @pearsonca ?

TimothyWillard · 2024-10-18T15:56:41Z

Still approve, but also still some weird whitespace diffs?

I'm also a bit confused about this one:

twillard@epid-iss-MacBook-Pro ~/D/G/H/flepiMoP (split-up-unit-tests-action)> git diff --name-only HEAD ( git merge-base main HEAD )
.github/workflows/ci.yml
.github/workflows/flepicommon-ci.yml
.github/workflows/gempyor-ci.yml
.github/workflows/inference-ci.yml

Changed the default python version to 3.11 from 3.11 after now having merged GH-280.

TimothyWillard requested review from pearsonca, jcblemai and shauntruelove August 2, 2024 18:30

TimothyWillard added 3 commits August 2, 2024 14:44

Correct working dir in inference-ci.yml

e3adcba

Typo in `setwd` call causes error about not being able to change to directory that doesn't exist.

Set gempyor integration tests shell

240b257

Set the shell to bash so the `source` function is available.

Limit paths for PRs in actions

e49109a

Add the same path related limits from the on push to on pull_requests as well.

pearsonca reviewed Aug 2, 2024

View reviewed changes

shauntruelove previously requested changes Aug 6, 2024

View reviewed changes

.github/workflows/flepicommon-ci.yml Outdated Show resolved Hide resolved

.github/workflows/gempyor-ci.yml Outdated Show resolved Hide resolved

Remove 'breaking-improvements' branch

49475af

Removed the `breaking-improvements` branch from special consideration in GitHub actions.

TimothyWillard requested a review from shauntruelove August 6, 2024 21:11

Merge branch 'main' into split-up-unit-tests-action

22ff4c9

TimothyWillard mentioned this pull request Aug 7, 2024

Initial python linting #282

Closed

Merge branch 'main' into split-up-unit-tests-action

6ac064f

jcblemai approved these changes Aug 12, 2024

View reviewed changes

TimothyWillard and others added 3 commits August 15, 2024 10:58

Merge main into split-up-unit-tests-action

24fdb86

Merge main into split-up-unit-tests-action

072dc27

Merge branch 'main' into split-up-unit-tests-action

b5c41cf

jcblemai previously approved these changes Oct 2, 2024

View reviewed changes

TimothyWillard mentioned this pull request Oct 3, 2024

Generic HPC Install Script #329

Merged

Update gempyor action to test 3.11 and 3.10

b56d2d9

Minor edits to run the gempyor tests with python 3.10 and 3.11. Remove usage of custom docker container.

TimothyWillard dismissed jcblemai’s stale review via b56d2d9 October 7, 2024 16:08

TimothyWillard added 2 commits October 7, 2024 12:22

Delete line to trigger GH Action

ea3ebc6

Very minor edit to `__init__.py` to trigger the `gempyor` CI GitHub action.

Correct extra install name

3d087c7

Need to install `test` extra installs to get pytest instead of `dev`.

TimothyWillard requested review from pearsonca and jcblemai October 7, 2024 16:34

TimothyWillard added 6 commits October 17, 2024 17:16

Add R version to cache

1ea7afe

Add back python 3.11

10b4a28

Remove whitespace to trigger full CI

6c9aff7

Rename 'ci' step to 'tests'

161915e

Merge main into split-up-unit-tests-action

d659cf7

Remove devtools::uninstall calls

0bebbb3

TimothyWillard requested review from pearsonca and jcblemai October 18, 2024 12:47

pearsonca previously approved these changes Oct 18, 2024

View reviewed changes

jcblemai previously approved these changes Oct 18, 2024

View reviewed changes

TimothyWillard added 2 commits October 18, 2024 11:10

Merge main into split-up-unit-tests-action

d1418de

Remove whitespace changes

cd81c5e

Remove the changes used to trigger the workflows as an example. Not needed for the PR.

TimothyWillard dismissed stale reviews from jcblemai and pearsonca via cd81c5e October 18, 2024 15:14

jcblemai previously approved these changes Oct 18, 2024

View reviewed changes

Remove dev branch from triggers

f67ffeb

TimothyWillard dismissed jcblemai’s stale review via f67ffeb October 18, 2024 15:26

jcblemai approved these changes Oct 18, 2024

View reviewed changes

jcblemai requested a review from pearsonca October 18, 2024 15:28

pearsonca approved these changes Oct 18, 2024

View reviewed changes

TimothyWillard merged commit 5f61657 into main Oct 18, 2024

TimothyWillard deleted the split-up-unit-tests-action branch October 18, 2024 15:56

TimothyWillard added a commit that referenced this pull request Oct 18, 2024

Change default python to 3.11

cbcfff2

Changed the default python version to 3.11 from 3.11 after now having merged GH-280.

jcblemai mentioned this pull request Oct 18, 2024

Split Up "unit-tests" GitHub Action Into Multiple Actions #278

Closed

TimothyWillard linked an issue Oct 18, 2024 that may be closed by this pull request

Split Up "unit-tests" GitHub Action Into Multiple Actions #278

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split `ci.yml` into separate actions #280

Split `ci.yml` into separate actions #280

TimothyWillard commented Aug 2, 2024

TimothyWillard commented Aug 2, 2024

TimothyWillard commented Aug 2, 2024

pearsonca left a comment

shauntruelove left a comment

TimothyWillard commented Aug 6, 2024

jcblemai commented Aug 12, 2024

TimothyWillard commented Oct 18, 2024 •

edited

Loading

pearsonca left a comment

pearsonca Oct 18, 2024

TimothyWillard Oct 18, 2024

TimothyWillard commented Oct 18, 2024

pearsonca left a comment

jcblemai commented Oct 18, 2024

TimothyWillard commented Oct 18, 2024

		@@ -28,6 +28,7 @@ cum_death_forecast <- function (sim_data,

		}

Split ci.yml into separate actions #280

Split ci.yml into separate actions #280

Conversation

TimothyWillard commented Aug 2, 2024

TimothyWillard commented Aug 2, 2024

TimothyWillard commented Aug 2, 2024

pearsonca left a comment

Choose a reason for hiding this comment

shauntruelove left a comment

Choose a reason for hiding this comment

TimothyWillard commented Aug 6, 2024

jcblemai commented Aug 12, 2024

TimothyWillard commented Oct 18, 2024 • edited Loading

pearsonca left a comment

Choose a reason for hiding this comment

pearsonca Oct 18, 2024

Choose a reason for hiding this comment

TimothyWillard Oct 18, 2024

Choose a reason for hiding this comment

TimothyWillard commented Oct 18, 2024

pearsonca left a comment

Choose a reason for hiding this comment

jcblemai commented Oct 18, 2024

TimothyWillard commented Oct 18, 2024

Split `ci.yml` into separate actions #280

Split `ci.yml` into separate actions #280

TimothyWillard commented Oct 18, 2024 •

edited

Loading