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

tests: properly build snapd snap #14141

Conversation

valentindavid
Copy link
Contributor

Now we build also the test version of snapd snap in snap-builds workflow job. We copy this into the spread tests. And we use that snap, which we only instrument instead of copying the snapd deb build.

If the snap is not available, then we build it in spread. On CI, this happens on arm since the workflow does not build it. It will also happen when triggering test manually.

@valentindavid valentindavid added the Run nested The PR also runs tests inluded in nested suite label Jun 28, 2024
@github-actions github-actions bot added the Run Nested -auto- Label automatically added in case nested tests need to be executed label Jun 28, 2024
@valentindavid
Copy link
Contributor Author

Github had a bug, and I had to re-create #13517.

snap download --basename=pc-kernel --channel="$CHANNEL/${KERNEL_CHANNEL}" pc-kernel
unsquashfs -no-progress -d "$UNPACK_DIR" pc-kernel.snap
snap pack --filename="$TARGET" "$UNPACK_DIR"
build_snapd_snap() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

At some point we should move this to a tool, but it is ok to have it in prepare.sh by now.

CHANNEL=latest
else
CHANNEL=$VERSION
run_snapcraft() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps the function could receive the path, so then we can use that to build other snaps too.


local UNPACK_DIR="/tmp/snapd-unpack"
unsquashfs -no-progress -d "$UNPACK_DIR" snapd_*.snap
TARGET="${1}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing local?

Copy link
Member

Choose a reason for hiding this comment

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

target is defined as local two lines up

Copy link
Collaborator

@sergiocazzolato sergiocazzolato left a comment

Choose a reason for hiding this comment

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

Nice, minor comments inline

Thanks for this change

@Meulengracht Meulengracht self-assigned this Jul 3, 2024
valentindavid and others added 2 commits July 3, 2024 09:58
Now we build also the test version of snapd snap in `snap-builds`
workflow job. We copy this into the spread tests. And we use that
snap, which we only instrument instead of copying the snapd deb build.

If the snap is not available, then we build it in spread. On CI, this
happens on arm since the workflow does not build it. It will also
happen when triggering test manually.
…o script scope, use PWD instead of dot notation
@Meulengracht Meulengracht force-pushed the valentindavid/with-patchelf-run-tests branch from 794b61a to 6cbcb47 Compare July 3, 2024 08:04
Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -9,6 +9,7 @@ set -eux
# shellcheck source=tests/lib/state.sh
. "$TESTSLIB/state.sh"

: "${WORK_DIR:=/tmp/work-dir}"
Copy link
Contributor

Choose a reason for hiding this comment

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

that's sneaky 😄

@Meulengracht Meulengracht merged commit b1dc3c6 into canonical:master Jul 3, 2024
46 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Nested -auto- Label automatically added in case nested tests need to be executed Run nested The PR also runs tests inluded in nested suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants