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

[skip-ci] Packit/TMT: Run gating tests #1960

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .fmf/version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
39 changes: 36 additions & 3 deletions .packit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
notifications: &copr_build_failure_notification
failure_comment:
message: "Ephemeral COPR build failed. @containers/packit-build please check."
targets:
targets: &fedora_copr_targets
Copy link
Contributor

Choose a reason for hiding this comment

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

This, and centos_copr_targets, seem to be unused. Is that intentional?

- fedora-development-x86_64
- fedora-development-aarch64
- fedora-latest-x86_64
Expand All @@ -43,7 +43,7 @@ jobs:
enable_net: true

- job: copr_build
trigger: pull_request
trigger: ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this an intentional part of the PR? It’s not obvious to me that changes to tests should affect whether we test building.

packages: [skopeo-eln]
notifications: *copr_build_failure_notification
targets:
Expand All @@ -59,7 +59,7 @@ jobs:
trigger: pull_request
packages: [skopeo-centos]
notifications: *copr_build_failure_notification
targets:
targets: &centos_copr_targets
- centos-stream-9-x86_64
- centos-stream-9-aarch64
- centos-stream-10-x86_64
Expand Down Expand Up @@ -88,6 +88,39 @@ jobs:
project: podman-next
enable_net: true

# Tests on Fedora
- job: tests
trigger: pull_request
packages: [skopeo-fedora]
notifications: &test_failure_notification
failure_comment:
message: "Tests failed. @containers/packit-build please check."
tmt_plan: /plans/upstream
targets:
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: Per https://packit.dev/docs/configuration/upstream/tests#required-parameters , targets which don’t specify an architecture imply x86_64 when used in tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth a comment in the file? Especially when it differs from the build job semantics.

- fedora-development
- fedora-latest
- fedora-latest-stable
- fedora-40
tf_extra_params:
environments:
- artifacts:
- type: repository-file
id: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/repo/fedora-$releasever/rhcontainerbot-podman-next-fedora-$releasever.repo

# Tests on CentOS Stream
- job: tests
trigger: pull_request
packages: [skopeo-centos]
notifications: *test_failure_notification
targets:
- centos-stream-9
- centos-stream-10
tf_extra_params:
environments:
- artifacts:
- type: repository-file
id: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/repo/centos-stream-$releasever/rhcontainerbot-podman-next-centos-stream-$releasever.repo

# Sync to Fedora
- job: propose_downstream
trigger: release
Expand Down
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,12 @@ test-integration:


# Intended for CI, assumed to be running in quay.io/libpod/skopeo_cidev container.
ifdef SKOPEO_BINARY
$(info Skipping build as SKOPEO_BINARY is specified)
test-integration-local:
else
test-integration-local: bin/skopeo
endif
Comment on lines +205 to +210
Copy link
Contributor

Choose a reason for hiding this comment

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

This suggests that SKOPEO_BINARY might be set. (I can’t see anything setting it. for integration tests.)

But the Go test code invoked by go test just assumes Skopeo is found in PATH, and noting sets up SKOPEO_BINARY to be handled.

How is this intended to work?

hack/warn-destructive-tests.sh
hack/test-integration.sh

Expand All @@ -228,7 +233,7 @@ test-unit:
$(CONTAINER_RUN) $(MAKE) test-unit-local

validate:
$(CONTAINER_RUN) $(MAKE) validate-local
$(CONTAINER_RUN) $(MAKE) tools validate-local

# This target is only intended for development, e.g. executing it from an IDE. Use (make test) for CI or pre-release testing.
test-all-local: validate-local validate-docs test-unit-local
Expand Down
8 changes: 5 additions & 3 deletions hack/test-integration.sh
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#!/bin/bash
set -e
set -exo pipefail

make PREFIX=/usr install
if [[ ! -f /usr/bin/skopeo ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAICS this introduces a risk that we are going to test some pre-existing system binary, not the one we just built (or, see elsewhere about SKOPEO_BINARY), and that it is not caller-determined for certain, but it depends on what happens to be in the environment. That ambiguity seems undesirable to me.

make PREFIX=/usr install
fi

echo "cd ./integration;" go test $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}
cd ./integration
go test $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}
go test $TESTFLAGS ${BUILDTAGS:+-tags "$BUILDTAGS"}
13 changes: 13 additions & 0 deletions hack/tmt/main.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
require:
- make
- podman-docker

/unit:
tag: upstream
summary: Run unit test
test: make -C ../.. test-unit

/validate:
tag: upstream
summary: Run validate test
test: make -C ../.. tools validate
11 changes: 11 additions & 0 deletions integration/tmt/main.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require:
- golang
- make
- podman
- skopeo
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this requiring some other Skopeo binary than the one we are testing? Or does that guarantee to use the one from copr_build?

… and if this is the one we should be testing, how does that /usr/bin/skopeo get injected into the test-integration container? That doesn’t happen AFAICS.


duration: 30m

tag: [ upstream, downstream ]
summary: Run integration tests
test: make -C ../.. test-integration
20 changes: 20 additions & 0 deletions plans/main.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
discover:
how: fmf
execute:
how: tmt

/upstream:
summary: Run tests on upstream PRs
discover+:
filter: tag:upstream
adjust+:
enabled: false
when: initiator is not defined or initiator != packit

/downstream:
summary: Run tests on bodhi / errata and dist-git PRs
discover+:
filter: tag:downstream
adjust+:
enabled: false
when: initiator == packit
2 changes: 1 addition & 1 deletion rpm/skopeo.spec
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ BuildRequires: ostree-devel
BuildRequires: glib2-devel
BuildRequires: make
BuildRequires: shadow-utils-subid-devel
Requires: containers-common >= 4:1-21
Requires: containers-common
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this related to the rest of the PR?


%description
Command line utility to inspect images and repositories directly on Docker
Expand Down