From 29ac9c191966515192372472c7a1f22304352159 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Fri, 19 Sep 2025 16:02:13 -0500 Subject: [PATCH 1/5] system-test: Fix crash when a comparison with a non-string value fails `orig` isn't necessarily a string, so the string concatenation here can raise a TypeError. --- system/lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/lib.py b/system/lib.py index f7761997..77670c86 100644 --- a/system/lib.py +++ b/system/lib.py @@ -517,7 +517,7 @@ class BaseTest(object): if gold != output: diff = "".join(difflib.unified_diff( [l + "\n" for l in gold.split("\n")], [l + "\n" for l in output.split("\n")])) - raise Exception("content doesn't match:\n" + diff + "\n\nOutput:\n" + orig + "\n") + raise Exception(f"content doesn't match:\n{diff}\n\nOutput:\n{orig}\n") check = check_output From ddf415a35916c5ac0bad918f9d5f7ff90b310da1 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Fri, 19 Sep 2025 16:02:13 -0500 Subject: [PATCH 2/5] docker: Fix usage with rootless podman and SELinux When using rootless podman, the *current user* gets mapped to uid 0, which results in the aptly user being unable to write to the build directory. We can instead map the current user to the corresponding uid in the container via `PODMAN_USERNS=keep-id`, which matches up with what docker-wrapper wants...but then that will *enter the container as the current uid*, which messes with the ability to set permissions on `/var/lib/aptly`. That can be fixed by explicitly passing `--user 0:0`, which should be a no-op on docker (since the container's default user is already root). Additionally, this adds `--security-opt label=disable` to avoid permission errors when running on systems with SELinux enforcing. --- Makefile | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 4692dfde..00293c2c 100644 --- a/Makefile +++ b/Makefile @@ -7,6 +7,9 @@ COVERAGE_DIR?=$(shell mktemp -d) GOOS=$(shell go env GOHOSTOS) GOARCH=$(shell go env GOHOSTARCH) +export PODMAN_USERNS = keep-id +DOCKER_RUN = docker run --security-opt label=disable -it --user 0:0 --rm -v ${PWD}:/work/src + # Setting TZ for certificates export TZ=UTC # Unit Tests and some sysmte tests rely on expired certificates, turn back the time @@ -173,16 +176,16 @@ docker-image-no-cache: ## Build aptly-dev docker image (no cache) @docker build --no-cache -f system/Dockerfile . -t aptly-dev docker-build: ## Build aptly in docker container - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper build + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper build docker-shell: ## Run aptly and other commands in docker container - @docker run -it --rm -p 3142:3142 -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper || true + @$(DOCKER_RUN) -p 3142:3142 aptly-dev /work/src/system/docker-wrapper || true docker-deb: ## Build debian packages in docker container - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64 + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper dpkg DEBARCH=amd64 docker-unit-test: ## Run unit tests in docker container (add TEST=regex to specify which tests to run) - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper \ + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \ azurite-start \ AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \ AZURE_STORAGE_ACCOUNT=devstoreaccount1 \ @@ -191,7 +194,7 @@ docker-unit-test: ## Run unit tests in docker container (add TEST=regex to spec azurite-stop docker-system-test: ## Run system tests in docker container (add TEST=t04_mirror or TEST=UpdateMirror26Test to run only specific tests) - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper \ + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper \ azurite-start \ AZURE_STORAGE_ENDPOINT=http://127.0.0.1:10000/devstoreaccount1 \ AZURE_STORAGE_ACCOUNT=devstoreaccount1 \ @@ -202,16 +205,16 @@ docker-system-test: ## Run system tests in docker container (add TEST=t04_mirro azurite-stop docker-serve: ## Run development server (auto recompiling) on http://localhost:3142 - @docker run -it --rm -p 3142:3142 -v ${PWD}:/work/src -v /tmp/cache-go-aptly:/var/lib/aptly/.cache/go-build aptly-dev /work/src/system/docker-wrapper serve || true + @$(DOCKER_RUN) -p 3142:3142 -v /tmp/cache-go-aptly:/var/lib/aptly/.cache/go-build aptly-dev /work/src/system/docker-wrapper serve || true docker-lint: ## Run golangci-lint in docker container - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper lint + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper lint docker-binaries: ## Build binary releases (FreeBSD, macOS, Linux generic) in docker container - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper binaries + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper binaries docker-man: ## Create man page in docker container - @docker run -it --rm -v ${PWD}:/work/src aptly-dev /work/src/system/docker-wrapper man + @$(DOCKER_RUN) aptly-dev /work/src/system/docker-wrapper man mem.png: mem.dat mem.gp gnuplot mem.gp From 568a9ce4d51376c55d47b7b551ce6295b7005660 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Fri, 19 Sep 2025 16:02:13 -0500 Subject: [PATCH 3/5] docker: Preserve the go build cache Otherwise, every `make docker-...` invocation will need to rebuild everything from scratch. --- system/docker-wrapper | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/docker-wrapper b/system/docker-wrapper index 1d92f392..a1396616 100755 --- a/system/docker-wrapper +++ b/system/docker-wrapper @@ -15,4 +15,4 @@ else fi cd /work/src -sudo -u aptly PATH=$PATH:/work/src/build GOPATH=/work/src/.go $cmd +sudo -u aptly PATH=$PATH:/work/src/build GOPATH=/work/src/.go GOCACHE=/work/src/.go/cache $cmd From 10f942c8e095f0cd11b9e3d1c9e5592b335b5438 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Fri, 19 Sep 2025 16:02:13 -0500 Subject: [PATCH 4/5] system-test: Forward CAPTURE to docker The code was only forwarding TEST, but CAPTURE is useful too. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 00293c2c..3e293292 100644 --- a/Makefile +++ b/Makefile @@ -201,7 +201,7 @@ docker-system-test: ## Run system tests in docker container (add TEST=t04_mirro AZURE_STORAGE_ACCESS_KEY="Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==" \ AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \ AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \ - system-test TEST=$(TEST) \ + system-test TEST=$(TEST) CAPTURE=$(CAPTURE) \ azurite-stop docker-serve: ## Run development server (auto recompiling) on http://localhost:3142 From 33a2f70d0707a597c584dfebd324b88531c2567e Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Fri, 19 Sep 2025 16:02:13 -0500 Subject: [PATCH 5/5] system-test: Allow skipping coverage Enabling coverage near-doubles the incremental build time and adds overhead to individual tests on the order of **5-10x** or more. It's not essential to have this for quick local system-test runs, so add an option to disable it. --- Makefile | 15 ++++++++++++--- system/lib.py | 6 ++++-- system/run.py | 9 ++++++--- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 3e293292..c8b7628d 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,15 @@ export TZ=UTC # Unit Tests and some sysmte tests rely on expired certificates, turn back the time export TEST_FAKETIME := 2025-01-02 03:04:05 +# run with 'COVERAGE_SKIP=1' to skip coverage checks during system tests +ifeq ($(COVERAGE_SKIP),1) +COVERAGE_ARG_BUILD := +COVERAGE_ARG_TEST := --coverage-skip +else +COVERAGE_ARG_BUILD := -coverpkg="./..." +COVERAGE_ARG_TEST := --coverage-dir $(COVERAGE_DIR) +endif + # export CAPUTRE=1 for regenrating test gold files ifeq ($(CAPTURE),1) CAPTURE_ARG := --capture @@ -106,13 +115,13 @@ test: prepare swagger etcd-install ## Run unit tests (add TEST=regex to specify system-test: prepare swagger etcd-install ## Run system tests # build coverage binary - go test -v -coverpkg="./..." -c -tags testruncli + go test -v $(COVERAGE_ARG_BUILD) -c -tags testruncli # Download fixture-db, fixture-pool, etcd.db if [ ! -e ~/aptly-fixture-db ]; then git clone https://github.com/aptly-dev/aptly-fixture-db.git ~/aptly-fixture-db/; fi if [ ! -e ~/aptly-fixture-pool ]; then git clone https://github.com/aptly-dev/aptly-fixture-pool.git ~/aptly-fixture-pool/; fi test -f ~/etcd.db || (curl -o ~/etcd.db.xz http://repo.aptly.info/system-tests/etcd.db.xz && xz -d ~/etcd.db.xz) # Run system tests - PATH=$(BINPATH)/:$(PATH) FORCE_COLOR=1 $(PYTHON) system/run.py --long --coverage-dir $(COVERAGE_DIR) $(CAPTURE_ARG) $(TEST) + PATH=$(BINPATH)/:$(PATH) FORCE_COLOR=1 $(PYTHON) system/run.py --long $(COVERAGE_ARG_TEST) $(CAPTURE_ARG) $(TEST) bench: @echo "\e[33m\e[1mRunning benchmark ...\e[0m" @@ -201,7 +210,7 @@ docker-system-test: ## Run system tests in docker container (add TEST=t04_mirro AZURE_STORAGE_ACCESS_KEY="Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==" \ AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \ AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \ - system-test TEST=$(TEST) CAPTURE=$(CAPTURE) \ + system-test TEST=$(TEST) CAPTURE=$(CAPTURE) COVERAGE_SKIP=$(COVERAGE_SKIP) \ azurite-stop docker-serve: ## Run development server (auto recompiling) on http://localhost:3142 diff --git a/system/lib.py b/system/lib.py index 77670c86..ff9c890c 100644 --- a/system/lib.py +++ b/system/lib.py @@ -310,7 +310,9 @@ class BaseTest(object): if command[0] == "aptly": aptly_testing_bin = Path(__file__).parent / ".." / "aptly.test" - command = [str(aptly_testing_bin), f"-test.coverprofile={Path(self.coverage_dir) / self.__class__.__name__}-{uuid4()}.out", *command[1:]] + command = [str(aptly_testing_bin), *command[1:]] + if self.coverage_dir is not None: + command.insert(1, f"-test.coverprofile={Path(self.coverage_dir) / self.__class__.__name__}-{uuid4()}.out") if self.faketime: command = ["faketime", os.environ.get("TEST_FAKETIME", "2025-01-02 03:04:05")] + command @@ -337,7 +339,7 @@ class BaseTest(object): if is_aptly_command: # remove the last two rows as go tests always print PASS/FAIL and coverage in those # two lines. This would otherwise fail the tests as they would not match gold - matches = re.findall(r"((.|\n)*)EXIT: (\d)\n.*\ncoverage: .*", raw_output) + matches = re.findall(r"((.|\n)*)EXIT: (\d)\n.*(?:\ncoverage: .*|$)", raw_output) if not matches: raise Exception("no matches found in command output '%s'" % raw_output) diff --git a/system/run.py b/system/run.py index 4e73fb2d..2b0de524 100755 --- a/system/run.py +++ b/system/run.py @@ -36,7 +36,7 @@ def natural_key(string_): return [int(s) if s.isdigit() else s for s in re.split(r'(\d+)', string_)] -def run(include_long_tests=False, capture_results=False, tests=None, filters=None, coverage_dir=None): +def run(include_long_tests=False, capture_results=False, tests=None, filters=None, coverage_dir=None, coverage_skip=False): """ Run system test. """ @@ -47,7 +47,7 @@ def run(include_long_tests=False, capture_results=False, tests=None, filters=Non fails = [] numTests = numFailed = numSkipped = 0 lastBase = None - if not coverage_dir: + if not coverage_dir and not coverage_skip: coverage_dir = mkdtemp(suffix="aptly-coverage") failed = False @@ -213,6 +213,7 @@ if __name__ == "__main__": include_long_tests = False capture_results = False coverage_dir = None + coverage_skip = False tests = None args = sys.argv[1:] @@ -224,6 +225,8 @@ if __name__ == "__main__": elif args[0] == "--coverage-dir": coverage_dir = args[1] args = args[1:] + elif args[0] == "--coverage-skip": + coverage_skip = True args = args[1:] @@ -236,4 +239,4 @@ if __name__ == "__main__": else: filters.append(arg) - run(include_long_tests, capture_results, tests, filters, coverage_dir) + run(include_long_tests, capture_results, tests, filters, coverage_dir, coverage_skip)