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

add script, which set QED dependencies of QED dependencies to the development version in integration tests #41

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

SimeonEhrig
Copy link
Member

As soon the PR is merged, other QED projects needs to rerun the CI pipeline to use the function.

fix #36

@SimeonEhrig
Copy link
Member Author

the test PR for this function is here: QEDjl-project/QEDcore.jl#35

@SimeonEhrig SimeonEhrig marked this pull request as draft July 15, 2024 15:28
@SimeonEhrig SimeonEhrig force-pushed the setDevDepDeps branch 3 times, most recently from 1d0c90f to 22f9099 Compare July 16, 2024 07:05
@SimeonEhrig SimeonEhrig marked this pull request as ready for review July 16, 2024 07:21
Copy link
Member

@AntonReinhard AntonReinhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good and fixes the issue. I think it's still necessary to keep the pipeline of integration tests that test against released version for merges to main, since those versions will themselves be released and need to work with the other registered packages.
The only exception would be releases to main with required simultaneous releases because of breaking changes.

@SimeonEhrig
Copy link
Member Author

This is good and fixes the issue. I think it's still necessary to keep the pipeline of integration tests that test against released version for merges to main, since those versions will themselves be released and need to work with the other registered packages.
The only exception would be releases to main with required simultaneous releases because of breaking changes.

I disagree because we only guaranty two thinks.

  • First if we release a QED package, which has dependency to other QED package, this dependencies are released versions.
  • Second, nearly all the time, the dev branches of all QED projects works together for a specific time point.

So the question is, if we test the our projects with the dev and the last release version of the dependencies, what we want to do, if the last release is not working? And also what is the benefit if we know that our current dev branch breaks with the release version of another package but not with the dev version?

@AntonReinhard
Copy link
Member

First if we release a QED package, which has dependency to other QED package, this dependencies are released versions.

Do we guarantee this currently? That's what I was suggesting, that we should test merges to main (which will be officially released right after) against the released versions of everything else, not their dev branches. If we do that already then I didn't realize that.

@SimeonEhrig
Copy link
Member Author

Do we guarantee this currently? That's what I was suggesting, that we should test merges to main (which will be officially released right after) against the released versions of everything else, not their dev branches. If we do that already then I didn't realize that.

Yes, because if we release we freeze all dev branches and create the new release version from it. The CI guaranties that all dev branches works together. There is only the exception, if a QED project has no changes and the dev branch is still equal to the last release. Therefore we do not need to do a release.

@SimeonEhrig
Copy link
Member Author

@AntonReinhard How continue with this PR? Uwe will be back in September.

@szabo137
Copy link
Member

szabo137 commented Jul 29, 2024

@SimeonEhrig I know we discussed the freeze of all devs for a full release in a very early stage of the project. However, at this point, I don't understand our conclusion from the past anymore 😅

Here are some thoughts from my side, how the integration tests should work (somewhat the dream state):

If a PR is opened targeting the main branch of package (which is most certainly a release or a hotfix), the integration tests should run for the released versions of the downstream packages. The reason is simple: if the PR is merged, it is the release and we want to guarantee, that all released versions work together properly. If these integration tests fail, two workflows are conceivable:

  1. The fix can be done directly on the release branch, which is fine, because all changes there are ported back to the dev anyway.
  2. The fix needs to be done downstream, which gives the developer an order of releases, most certainly the dependency graph.

If a PR is opened targeting dev, the integration tests should run for the dev versions of the downstream packages. Running against main, i.e. the released versions, is not necessary, because this will be done during the release pipeline anyways (see above). If these tests break, one needs to proceed with the fixing workflow written in the docs: fixing on a branch of the downstream package and using commit messages indicating the fix to the upstream PR.

These two pipelines are somewhat similar, but the difference is: the first workflow is very strict with fixes to guarantee interoperability of released versions. The second (currently used) workflow has some states, where packages do not work together, e.g. of the downstream fix is not merged yet, which is fine for dev branches. Side note: if for a packages, there are no stages changes, dev and main are in sync, therefore testing against dev is testing against release.

@AntonReinhard @SimeonEhrig what do you think?

@AntonReinhard
Copy link
Member

I agree. I think the problem with freezing the dev branches is that we would have to release every package every time we want to release any package. And releasing does take some time, plus it would probably end up bloating the versions unnecessarily.

The fix needs to be done downstream, which gives the developer an order of releases, most certainly the dependency graph.

As you say later, this means we will still need the option to change the branch we test against for merges to main, the same way it currently works with merges to dev.

@szabo137
Copy link
Member

As you say later, this means we will still need the option to change the branch we test against for merges to main, the same way it currently works with merges to dev.

Actually no, the tests for PRs targeting main should always test against released versions. In this regard, fails of the tests always cause a suspension of the release till the incompatible/breaking parts are fixed and the PR to main can be merged without any side effects.

@AntonReinhard
Copy link
Member

Actually no, the tests for PRs targeting main should always test against released versions. In this regard, fails of the tests always cause a suspension of the release till the incompatible/breaking parts are fixed and the PR to main can be merged without any side effects.

Okay, that just means then that sometimes it will be somewhat difficult to release (as with the QEDcore move), but it should always be possible, you're right.

So then @SimeonEhrig I think all that should be done here is to have two separate pipelines, where for

  • merges to main, everything is tested against all other released versions, and for
  • merges to dev, everything is tested against all other dev versions
    instead of always testing against dev versions, as it is now.

@SimeonEhrig
Copy link
Member Author

Okay, if the target branch is main test against the released version from the registry, else against the dev branch (we need also a solution, if we merge in another feature branch).

@szabo137
Copy link
Member

... (we need also a solution, if we merge in another feature branch).

What do you mean with that? We already have workflow to handle feature branches, or am I wrong? 🤔

@SimeonEhrig
Copy link
Member Author

... (we need also a solution, if we merge in another feature branch).

What do you mean with that? We already have workflow to handle feature branches, or am I wrong? 🤔

I mean, if you want to merge a feature branch in another feature branch and not the dev branch.

@szabo137
Copy link
Member

... (we need also a solution, if we merge in another feature branch).

What do you mean with that? We already have workflow to handle feature branches, or am I wrong? 🤔

I mean, if you want to merge a feature branch in another feature branch and not the dev branch.

In the current contribution policy, this could only happen on a fork, not in the main repo. I think for such cases, we don't provide CI support?

@SimeonEhrig
Copy link
Member Author

In the current contribution policy, this could only happen on a fork, not in the main repo. I think for such cases, we don't provide CI support?

It's more a implementation question. Ether I implement

if $TARGET_BRANCH == "main"
  // use registered packages
else
  // use dev version

or

if $TARGET_BRANCH == "main"
  // use registered packages
elif $TARGET_BRANCH == "dev"
  // use dev version
else
  error("unsupported")

@AntonReinhard
Copy link
Member

I think the first option makes the most sense. Use registered packages for PRs to main and dev otherwise.

…cies to dev branch version depending on the target branch

- if the target branch is main, use released QED dependencies
- if the target branch is not main, use the dev branch versions of the QED dependencies
- add also an script to determine the PR target branch in the GitLab CI
@SimeonEhrig
Copy link
Member Author

The script is used in other repositories to generated integration tests. Therefore I need to open PRs in another repositories to test it.

AntonReinhard
AntonReinhard previously approved these changes Aug 28, 2024
Copy link
Member

@AntonReinhard AntonReinhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SimeonEhrig SimeonEhrig merged commit b1e1983 into QEDjl-project:dev Aug 29, 2024
3 checks passed
@AntonReinhard AntonReinhard added this to the Release-0.1.0 milestone Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration test: check against devs
3 participants