Coverage only runs runs when meaningful files are changed

okBrian commented 3 weeks ago

Description

Added https://github.com/dorny/paths-filter to CI based on @sbryngelson Pull Request #461 The Filter is modulated on purpose for future use as you can specify which grouping you want as seen in test.yml & coverage.yml

Type of change

[x] Something else

Scope

[x] This PR comprises a set of related changes with a common goal

If you cannot check the above box, please split your PR into multiple PRs that each have a common goal.

How Has This Been Tested?

[x] Edited the CMakeFile, then tested to make sure the filter caught the changes, and the tests would run.
[x] Edited no files and sent changes and commented away some parts of the filter to ensure the test would be skipped.

Test Configuration:

Github Actions on my fork

Checklist

[x] I have added comments for the new code
[x] I ran ./mfc.sh format before committing my code
[x] This PR does not introduce any repeated code (it follows the DRY principle)
[x] I cannot think of a way to condense this code and reduce any introduced additional line count

sbryngelson commented 3 weeks ago

can be reviewed and merged by @henryleberre

codecov[bot] commented 3 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 57.91%. Comparing base (596ef8b) to head (08262e1).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #468 +/- ## ======================================= Coverage 57.91% 57.91% ======================================= Files 55 55 Lines 14230 14230 Branches 1854 1854 ======================================= Hits 8242 8242 Misses 5452 5452 Partials 536 536 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

henryleberre commented 3 weeks ago

Is there a reason you can't use the paths: feature like we do for on: [push] ?

okBrian commented 3 weeks ago

The main reason I suggested using https://github.com/dorny/paths-filter was because the path applied to pull request, push, and workflow_dispatch. I don't think their is a way to do something like this

on: 
  [push, pull_request, workflow_dispatch]
      paths:
      - '**.f90'
      - '**.fpp'
      - '**.py'
      - '**.yml'
      - 'mfc.sh'
      - 'golden.txt'
      - 'CMakeLists.txt'
      - 'requirements.txt'

If we can't do something similar to above we are repeating a lot of code, which is something Spencer was trying to change in his pr I think. Also since the path filter can read from file all CI can share the same file (like coverage and test) and just filter what it needs.

sbryngelson commented 3 weeks ago

indeed the above syntax doesn't work, though I do notice that you have to repeat your filtering statement in all of the workflows in tests.yml ...

okBrian commented 3 weeks ago

This is true, but in the future if we have different jobs in the same files that want to check different files we can specify that it in the conditional like in this example: https://github.com/GoogleChrome/web.dev/blob/3a57b721e7df6fc52172f676ca68d16153bda6a3/.github/workflows/lint-workflow.yml#L26 . This feature might be useful if we add more complex tests.

henryleberre commented 2 weeks ago

I was assuming that Github supported YAML anchors.. but they don't. https://github.com/actions/runner/issues/1182 has been open since 2021 and has 1k upvotes. The last comment in that thread gives an example of how I was imagining using it.

MFlowCode / MFC