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.

Sign up for GitHub

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

update to ginkgo v2 #18163

Merged
merged 23 commits into from
May 2, 2023
+39,245 −20,537
Merged

update to ginkgo v2 #18163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4458150
update to ginkgo v2
Luap99 Apr 12, 2023
666e314
ginkgo v2: remove deprecated flags
Luap99 Apr 12, 2023
c564d9d
ginkgo v2: remove CurrentGinkgoTestDescription()
Luap99 Apr 12, 2023
fbb7c98
test/e2e: containers.conf tests add missing Wait()
Luap99 Apr 12, 2023
b9ba850
Lower e2e timeout to not waste time when it hangs
Luap99 Apr 12, 2023
629a6a6
test/e2e: actually check for cleanup errors
Luap99 Apr 12, 2023
9bd833b
test/e2e: fix "podman run ipcns ipcmk container test"
Luap99 Apr 12, 2023
9c1f713
test/e2e: fix Cleanup()
Luap99 Apr 13, 2023
8cc7a36
ginkgo: run on all cores
Luap99 Apr 13, 2023
cc19091
test/e2e: unset CONTAINERS_CONF before Cleanup()
Luap99 Apr 13, 2023
fb7a966
test/e2e: switch to GinkgoWriter
Luap99 Apr 13, 2023
2ce4e93
ginkgo v2: drop localbenchmarks
Luap99 Apr 13, 2023
cd46e72
test/e2e: fix pause tests to unpause before cleanup()
Luap99 Apr 13, 2023
bc1ed07
ginkgo v2: fix new Skip() behavior
Luap99 Apr 13, 2023
c5922cc
test/e2e: fix CleanupVolume/Secrets()
Luap99 Apr 14, 2023
054154c
test/e2e: run system reset test serial
Luap99 Apr 17, 2023
b3424f3
test/e2e: unshare --rootless-netns cleanup slirp4netns
Luap99 Apr 18, 2023
914ff01
WIP: logformatter: handle ginkgo v2 logs
edsantiago Apr 13, 2023
1bff010
logformatter: anchors: link to test summary, not name
edsantiago Apr 18, 2023
5eb99a0
test/e2e: fix custom timing reporting
Luap99 Apr 21, 2023
c4b9f4b
ginkgo-v2 cleanup workaround for #18180
Luap99 Apr 28, 2023
5af4339
pkg/machine/e2e: switch to GinkgoWriter
Luap99 May 2, 2023
3858a83
Makefile: do not run machine test in parallel
Luap99 May 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
6 changes: 3 additions & 3 deletions .cirrus.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ local_integration_test_task: &local_integration_test_task
- unit_test
matrix: *platform_axis
gce_instance: *standardvm
timeout_in: 90m
timeout_in: 50m
cevich marked this conversation as resolved.
Show resolved Hide resolved
env:
TEST_FLAVOR: int
clone_script: *get_gosrc
Expand Down Expand Up @@ -620,7 +620,7 @@ container_integration_test_task:
CI_DESIRED_RUNTIME: crun
CI_DESIRED_NETWORK: cni
gce_instance: *standardvm
timeout_in: 90m
timeout_in: 50m
env:
TEST_FLAVOR: int
TEST_ENVIRON: container
Expand All @@ -639,7 +639,7 @@ rootless_integration_test_task:
depends_on: *build_unit
matrix: *platform_axis
gce_instance: *standardvm
timeout_in: 90m
timeout_in: 50m
env:
TEST_FLAVOR: int
PRIV_NAME: rootless
Expand Down
23 changes: 7 additions & 16 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ LIBSECCOMP_COMMIT := v2.3.3
GINKGOTIMEOUT ?= -timeout=90m
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest setting this timeout LOWER than the one in .cirrus.yml. As you've seen, when the cirrus-ci one hits, it just kills the VM. We get no logs or anything. It would be good to allow ginkgo's timeout to fire first so potentially helpful output and logs can be collected.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes I am working on it but the hang bug still happens, i.e. ginkgo thinks it is done but some goroutine causes the process to hang and never exit.

Copy link
Member

@cevich cevich May 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Previously when there was a ginkgo hang, you could find out what it was by examining the ginkgo-node log files collected as CI-artifacts IIRC. However, if Cirrus times out the task, the logs won't be collected.

# By default, run test/e2e
GINKGOWHAT ?= test/e2e/.
# By default, run tests in parallel across 3 nodes.
GINKGONODES ?= 3
GINKGO_PARALLEL=y
GINKGO ?= ./test/tools/build/ginkgo

# Conditional required to produce empty-output if binary not built yet.
Expand Down Expand Up @@ -542,11 +541,11 @@ localunit: test/goecho/goecho test/version/version
UNIT=1 $(GINKGO) \
-r \
$(TESTFLAGS) \
--skipPackage test/e2e,pkg/apparmor,pkg/bindings,hack,pkg/machine/e2e \
--skip-package test/e2e,pkg/bindings,hack,pkg/machine/e2e \
--cover \
--covermode atomic \
--coverprofile coverprofile \
--outputdir ${COVERAGE_PATH} \
--output-dir ${COVERAGE_PATH} \
--tags "$(BUILDTAGS)" \
--succinct
$(GO) tool cover -html=${COVERAGE_PATH}/coverprofile -o ${COVERAGE_PATH}/coverage.html
Expand All @@ -558,8 +557,8 @@ test: localunit localintegration remoteintegration localsystem remotesystem ##

.PHONY: ginkgo-run
ginkgo-run: .install.ginkgo
ACK_GINKGO_RC=true $(GINKGO) version
ACK_GINKGO_RC=true $(GINKGO) -v $(TESTFLAGS) -tags "$(TAGS) remote" $(GINKGOTIMEOUT) -cover -flakeAttempts 3 -progress -trace -noColor -nodes $(GINKGONODES) -debug $(GINKGOWHAT) $(HACK)
Copy link
Member

Choose a reason for hiding this comment

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

Removing -cover looks correct to me: it's handled in the localunit target above. I've traced the presence of -cover in this line all the way back to 2018, and believe it to be a copy/paste from the 2017 CRI-O import. But since there's a slight possibility that I'm wrong, I'm calling attention to it anyway. If anyone sees a reason to keep this, please speak now.

$(GINKGO) version
$(GINKGO) -vv $(TESTFLAGS) --tags "$(TAGS) remote" $(GINKGOTIMEOUT) --flake-attempts 3 --trace --no-color $(if $(findstring y,$(GINKGO_PARALLEL)),-p,) $(GINKGOWHAT) $(HACK)

.PHONY: ginkgo
ginkgo:
Expand All @@ -571,7 +570,7 @@ ginkgo-remote:

.PHONY: testbindings
testbindings: .install.ginkgo
ACK_GINKGO_RC=true $(GINKGO) -v $(TESTFLAGS) -tags "$(TAGS) remote" $(GINKGOTIMEOUT) -progress -trace -noColor -debug -timeout 30m -v -r ./pkg/bindings/test
$(GINKGO) -v $(TESTFLAGS) --tags "$(TAGS) remote" $(GINKGOTIMEOUT) --trace --no-color --timeout 30m -v -r ./pkg/bindings/test
Copy link
Member

Choose a reason for hiding this comment

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

There's a $(GINKGOTIMEOUT) specified here in addition to a --timeout 30m, not sure if that was intentional / doing some tricky thing with ginkgo or not. Consider removing one or the other or adding a comment detailing why there are two timeout options.


.PHONY: localintegration
localintegration: test-binaries ginkgo
Expand All @@ -581,15 +580,7 @@ remoteintegration: test-binaries ginkgo-remote

.PHONY: localmachine
localmachine: test-binaries .install.ginkgo
$(MAKE) ginkgo-run GINKGONODES=1 GINKGOWHAT=pkg/machine/e2e/. HACK=

.PHONY: localbenchmarks
localbenchmarks: install.tools test-binaries
PATH=$(PATH):$(shell pwd)/hack ACK_GINKGO_RC=true $(GINKGO) \
-focus "Podman Benchmark Suite" \
-tags "$(BUILDTAGS) benchmarks" -noColor \
-noisySkippings=false -noisyPendings=false \
test/e2e/.
$(MAKE) ginkgo-run GINKGO_PARALLEL=n GINKGOWHAT=pkg/machine/e2e/. HACK=

.PHONY: localsystem
localsystem:
Expand Down
34 changes: 0 additions & 34 deletions contrib/cirrus/lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,37 +335,3 @@ remove_packaged_podman_files() {
# Be super extra sure and careful vs performant and completely safe
sync && echo 3 > /proc/sys/vm/drop_caches || true
}

# Execute make localbenchmarks in $CIRRUS_WORKING_DIR/data
# for preserving as a task artifact.
localbenchmarks() {
local datadir envnames envname
req_env_vars DISTRO_NV PODBIN_NAME PRIV_NAME TEST_ENVIRON TEST_FLAVOR
req_env_vars VM_IMAGE_NAME EC2_INST_TYPE

datadir=$CIRRUS_WORKING_DIR/data
mkdir -p $datadir

envnames=$(passthrough_envars | sort);
(
echo "# Env. var basis for benchmarks benchmarks."
for envname in $envnames; do
printf "$envname=%q\n" "${!envname}"
done

echo "# Machine details for data-comparison sake, not actual env. vars."
# Checked above in req_env_vars
# shellcheck disable=SC2154
echo "\
BENCH_ENV_VER=1
CPUTOTAL=$(grep -ce '^processor' /proc/cpuinfo)
INST_TYPE=$EC2_INST_TYPE
MEMTOTALKB=$(awk -F: '$1 == "MemTotal" { print $2 }' </proc/meminfo | sed -e "s/^ *//" | cut -d ' ' -f 1)
UNAME_R=$(uname -r)
UNAME_M=$(uname -m)
"
) > $datadir/benchmarks.env
make localbenchmarks | tee $datadir/benchmarks.raw
msg "Processing raw benchmarks output"
hack/parse-localbenchmarks < $datadir/benchmarks.raw | tee $datadir/benchmarks.csv
}
Loading