Only run test suite if code changes

[hoxbro]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1fe9b28) 84.67% compared to head (c8e3853) 84.66%.
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6096      +/-   ##
==========================================
- Coverage   84.67%   84.66%   -0.01%     
==========================================
  Files         296      296              
  Lines       44076    44096      +20     
==========================================
+ Hits        37320    37334      +14     
- Misses       6756     6762       +6     

Flag	Coverage Δ
ui-tests	40.68% <ø> (+0.02%)	⬆️
unitexamples-tests	72.67% <ø> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maximlt]

Isn't there a way instead to exclude files and folders? I find this a bit error prone. And I think you forgot /doc?

[hoxbro]

Isn't there a way instead to exclude files and folders? I find this a bit error prone.

I think this is more of a preference thing. I think being inclusive is a good thing. We can always add more items to a list if we find a missing folder or file. The test suite will still run most of the time.

And I think you forgot /doc?

There's a slight tension here; while it makes sense to run when docs with code change, most updates outside the codebase, happen in the doc folder. I have added a doc check. Right now I have only added a check for doc/how_to. More can be added.

[maximlt]

I think I disagree on both points :) On the first one, for instance you have not included tox.ini or dodo.py or setup.cfg while they may all affect how the tests run. When the CI runs I almost never check what it actually ran, I just rely on the green flag. And on the second one, I'd say all the runnable docs are important, not just the How-Tos. The getting started for instance is really important.

If the goal is to reduce the number of CI runs, I'd suggest considering no longer running the test suite after a MR is merged on main.

[hoxbro]

My list of files and folders is not exhaustive; as I wrote in my previous comment, it can always be expanded.

On the first one, for instance you have not included tox.ini or dodo.py or setup.cfg while they may all affect how the tests run.

tox.ini definitely could affect the test. The other two (in their current state) cannot. I didn't add tox.ini because we will likely remove it in the near future. This is not a very good reason, so I will add it to the list. The other two can be added if they are updated to affect the test suite in the future.

And on the second one, I'd say all the runnable docs are important, not just the How-Tos. The getting started for instance is really important. (emphasis mine)

I agree! I simply want to be more selective about it than adding the whole doc/ folder. I will add the getting started to the docs change section.

If the goal is to reduce the number of CI runs, I'd suggest considering no longer running the test suite after a MR is merged on main.

The goal is to reduce the number of unnecessary CI runs. What classifies as unnecessary is hard in non-concreate cases. We can all agree that it is stupid that the test CI runs when a changelog is updated.

I'm not sure if I want to disable running the test suite on main; it has advantages at the top of my I can think of two;

1. If multiple PRs are merged in rapid succession, the test suite running on the main would be able to catch it a combination of PRs breaks the CI.

   This is harder to detect for Panel test suite as we still have many flaky tests, making it hard not to become ci-failing-blind (ignoring failing tests as they are always failing anyway).

   The main branch also has a lot fewer test runs than runs made in PRs, where a single commit can have 5 or 10 test runs.

   I also like that the main branch conveys the "true" story about the state of the test suite, e.g., do our test work, or do we have a problem we need to fix?

2. Caching of packages. If no test runs have run on the main branch, each new PR will have to create its own cache [ref]. This is maybe not as "big" of a win.

[maximlt]

The other two (in their current state) cannot

I'm pretty sure I could break the test suite modifying one of these two files, don't challenge me :D

If multiple PRs are merged in rapid succession, the test suite running on the main would be able to catch it a combination of PRs breaks the CI.

I agree with that in theory, in practice, I think I've actually never seen that. It might well be that I missed it because I'm not set up to receive emails from Github when workflows fail (that was just too noisy), or it just never happened. I would say that running the CI on the main branch is definitely something you need to do when you're in a system that is continuously deployed, as you want to deploy after the test suite passes successfully. For packages that have point in time releases, that's more questionable.

Clearly we won't agree on this :)

[hoxbro]

I'm pretty sure I could break the test suite modifying one of these two files, don't challenge me :D

I know you can, but I hope that if you do something that malicious, you also update the list 🙃

I agree with that in theory, in practice, I think I've actually never seen

I would agree with that. I still think the workload on the main branch is significantly smaller than the workload across PRs. In the future, we could set up a nightly run to set up a cache each day and early detect problems with upstream releases. Maybe even have some kind of dashboard that can visualize successful/flaky/error runs. Can see dask does this.

Clearly we won't agree on this :)

😃

Like the discussion, though, it's always nice to have one's view challenged.

[philippjfr]

I'm fine with merging this. I'm unlikely not to notice if the tests didn't run when they should have.