Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse CI integration with downstream Ferrite packages #742

Open
termi-official opened this issue Jun 10, 2023 · 5 comments · May be fixed by #797
Open

Reverse CI integration with downstream Ferrite packages #742

termi-official opened this issue Jun 10, 2023 · 5 comments · May be fixed by #797
Assignees

Comments

@termi-official
Copy link
Member

We should add some basic integration testing for packages downstream of Ferrite core to catch internal problems earlier (currently FerriteViz.jl and FerriteDistributed.jl).

References

https://discourse.julialang.org/t/github-actions-workflow-for-reverse-dependency-integration-testing/53536

https://github.com/JuliaDiff/ChainRulesCore.jl/blob/caf8692ca1bf5fabeb4afc86340eef9456da469c/.github/workflows/IntegrationTest.yml#L65

@KnutAM
Copy link
Member

KnutAM commented Jun 10, 2023

currently FerriteViz.jl and FerriteDistributed.jl)

and FerriteGmsh.jl and FerriteMeshParser.jl

We should add some basic integration testing for packages downstream of Ferrite core

Yes, that would be nice for all such packages that are in the Ferrite-FEM org!
I'm not sure how this would work in practice. Currently, for example, we haven't changed to 0.4 in master, but included a lot of breaking changes. Perhaps this would be fine just setting the version to 0.4.0-dev - not sure what is the "correct" approach.
However, what happens when a sub-package fixes one breaking change, allows Ferrite 0.4.0-dev, and then another new breaking change is introduced. Then the workflow will fail for all new PR's until the downstream package is fixed. Is this the desired result? Or perhaps I'm misunderstanding how this would work?

An alternative approach is that downstream packages can run timed (e.g. weekly) CI-runs against Ferrite#master to be "notified" about breaking changes in Ferrite.jl, see e.g. https://github.com/KnutAM/FerriteAssembly.jl/blob/main/.github/workflows/FerriteMasterCI.yml

@termi-official
Copy link
Member Author

currently FerriteViz.jl and FerriteDistributed.jl)

and FerriteGmsh.jl and FerriteMeshParser.jl

What do you think is missing for basic test coverage in via what we have in docs? (assuming the porous media example will be merged soon)

We should add some basic integration testing for packages downstream of Ferrite core

Yes, that would be nice for all such packages that are in the Ferrite-FEM org! I'm not sure how this would work in practice. Currently, for example, we haven't changed to 0.4 in master, but included a lot of breaking changes. Perhaps this would be fine just setting the version to 0.4.0-dev - not sure what is the "correct" approach. However, what happens when a sub-package fixes one breaking change, allows Ferrite 0.4.0-dev, and then another new breaking change is introduced. Then the workflow will fail for all new PR's until the downstream package is fixed. Is this the desired result? Or perhaps I'm misunderstanding how this would work?

An alternative approach is that downstream packages can run timed (e.g. weekly) CI-runs against Ferrite#master to be "notified" about breaking changes in Ferrite.jl, see e.g. https://github.com/KnutAM/FerriteAssembly.jl/blob/main/.github/workflows/FerriteMasterCI.yml

Usually downstream packages are still bound to some Ferrite version, e.g. 0.3 and if we bump to e.g. 0.4 then there is no direct way of testing. In this case we will likely need some special test for master vs master to co-develop the releases.

@KnutAM
Copy link
Member

KnutAM commented Jun 10, 2023

What do you think is missing for basic test coverage in via what we have in docs? (assuming the porous media example will be merged soon)

True! If we want to use the docs for checking this, then I guess it's fine! With #517 FerriteViz would also be used in the docs, and I hope there will be a FerriteDistributed example as well :) (I think that would make sense to have in the Ferrite's docs). (I'm just realizing that this would give exactly the same issue that I was concerned with above, that docs will fail until the downstream packages include the fixes. And this would also mean that the doc should build using the main version of downstream packages, I think it runs with latest now, but haven't checked).

Usually downstream packages are still bound to some Ferrite version

Going from 0.3 to 0.4, it makes sense to co-release. But we probably want to avoid having to keep even minor versions in sync?

One option could be to have a release workflow, that should be run manually before releasing a new version of Ferrite, and this should run with both Ferrite#master vs downstream#main and Ferrite#master vs downstream#latest to see if there are any changes that must be addressed before releasing Ferrite?
Such a release workflow could also include the benchmarks perhaps. This way, we could catch performance regressions before releasing a new version?

@termi-official
Copy link
Member Author

True! If we want to use the docs for checking this, then I guess it's fine! With #517 FerriteViz would also be used in the docs, and I hope there will be a FerriteDistributed example as well :)

I am a bit concerned regarding the tight integration of FerriteViz in the docs, because we do not have a nice way to "skip failing examples" yet, which just leads to a full block in the CI. Also I have not thought about putting a FerriteDistributed example into the core docs, but in a separate CI (same with FerriteViz), fot he very same reason.

But we probably want to avoid having to keep even minor versions in sync?

Yes, this is just about breaking changes must come in some kind of pairing.

One option could be to have a release workflow, that should be run manually before releasing a new version of Ferrite, and this should run with both Ferrite#master vs downstream#main and Ferrite#master vs downstream#latest to see if there are any changes that must be addressed before releasing Ferrite?
Such a release workflow could also include the benchmarks perhaps. This way, we could catch performance regressions before releasing a new version?

I think this is very similar to what Fredrik and I had in mind here. :)

@KnutAM
Copy link
Member

KnutAM commented Jun 10, 2023

I think this is very similar to what Fredrik and I had in mind here. :)

Haha, great! Then I misunderstood - I thought the intention was to run on every PR :)

Also I have not thought about putting a FerriteDistributed example into the core docs, but in a separate CI (same with FerriteViz), fot he very same reason.

Good point, I don't know enough about FerriteViz/Distributed to tell if that would become a problem.

In any case, it could be nice to have the docs on the same page (but that doesn't mean it has to run in the same CI of course)

@termi-official termi-official linked a pull request Sep 20, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants