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

Run tests against snapd stable #87

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

valentindavid
Copy link
Collaborator

Otherwise we might end up merging things that depend on fixes of snapd that are not released.

@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch 3 times, most recently from 3868c21 to c24cbcd Compare November 14, 2022 15:10
@valentindavid valentindavid reopened this Nov 14, 2022
@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch 5 times, most recently from 288cd59 to 4559641 Compare November 14, 2022 15:23
@valentindavid valentindavid reopened this Nov 15, 2022
@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch 2 times, most recently from 49100f1 to c4f1084 Compare November 15, 2022 11:30
@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch from c4f1084 to a4e8956 Compare November 15, 2022 14:12
Copy link
Member

@Meulengracht Meulengracht left a comment

Choose a reason for hiding this comment

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

Thanks! I added a suggestion for the future in how to handle running tests with different variables, but I don't require you to change anything here.

@@ -39,88 +39,26 @@ jobs:
spread -discard -reuse-pid="$(echo "$r" | grep -o -E '[0-9]+')"
done

tests-main:
runs-on: self-hosted
tests-main-stable:
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for doing this! Looks nice, alternative idea for future times, is that you could use a test-matrix instead of defining identical targets where only a variable is changing:

See examples:
https://github.com/snapcore/core-testing-jobs/blob/main/.github/workflows/cdimage-nightly.yml

So you could do like this:


    strategy:
      fail-fast: false
      matrix:
        channel: [stable, edge]
    with:
      snapd_branch: ${{ matrix.channel }}

Even better, maybe you could even define the templates as as matrix variable, but I'm not sure that github would accept that, as I don't know the order of evaluation when the workflow is passed (it was a hell with azure actions).


    strategy:
      fail-fast: false
      matrix:
        workflow: [tests-main.yml, tests-snapd.yml]
        channel: [stable, edge]
    uses: ./.github/workflows/${{ matrix.workflow }}
    with:
      snapd_branch: ${{ matrix.channel }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That works, yes. We should do that in another PR. I am not totally yet convince we require a matrix here.

@alfonsosanchezbeato
Copy link
Member

This needs rebasing

@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch from a4e8956 to 9e334af Compare January 23, 2024 08:45
Otherwise we might end up merging things that depend on fixes of snapd
that are not released.
@valentindavid valentindavid force-pushed the valentindavid/check-with-stable branch from 9e334af to 4eb9bbc Compare January 23, 2024 11:23
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.

3 participants