ci: run the functional tests on macOS - #320
Draft
u5surf wants to merge 3 commits into
Draft
Conversation
The zone is declared as 64k, which is 16 slab pages on a 4k page system
but only 4 on a 16k one. nginx allocates a whole page per slot size, so
storing a key and a value of different size classes already needs two of
those four, and the third `shared_dict` write in the test fails:
slab alloc: 9 slot: 1
[crit] ngx_slab_alloc() failed: no memory
The example drops the write silently, so the request still returns 200
and three subtests fail on a value that was never stored. This is
reproducible on Apple Silicon macOS, and applies equally to Linux arm64
builds configured with 16k or 64k pages; only the 4k page systems CI
runs on have enough room.
512k keeps the zone at eight pages or more everywhere, matching the
minimum that nginx's own zone-backed modules enforce.
Signed-off-by: Y.Horie <u5.horie@gmail.com>
The awssig example depends on iana_time_zone, which resolves the system
time zone through CoreFoundation on Apple platforms. Nothing pulls the
framework into the link line, so building the examples as nginx modules
on macOS fails:
"_CFStringGetCStringPtr", referenced from:
iana_time_zone::platform::string_ref::StringRef$LT$T$GT$::as_utf8
in libawssig.a
ld: symbol(s) not found for architecture arm64
Add the framework to that module's libs on Darwin only; every other
platform keeps the current link line.
Signed-off-by: Y.Horie <u5.horie@gmail.com>
prove examples/t currently runs on the Linux and Windows jobs only, and both use 4k memory pages. The macOS job in ci.yaml builds and runs cargo test but never the functional suite, so no CI job exercises the tests on a system with larger pages. That gap hid a real failure: a 64k shared memory zone is sixteen slab pages at 4k but only four at 16k, which is not enough for the shared_dict test to store a key and a value in different size classes. Add a macOS job to the NGINX workflow, mirroring the Linux one with a single static configuration to keep the added time modest. Homebrew already provides the perl and prove that Test::Nginx needs. Signed-off-by: Y.Horie <u5.horie@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Draft, because it sits on top of two other PRs. The branch carries their commits so that CI can show the end state; only the third commit belongs to this PR.
test: enlarge the shared_dict zone for larger memory pagesfix(examples): link CoreFoundation for the awssig example on Darwinci: run the functional tests on macOSI will rebase and mark it ready once those two land. Happy to close it in the meantime if you would rather not have a draft sitting open.
Proposed changes
prove examples/truns on the Linux and Windows jobs only. Both use 4k memory pages, and the macOS job inci.yamlrunscargo testwithout ever configuring nginx with--add-module. So no job exercises the functional suite on a system whose page size differs from 4k.That gap hid a real failure. nginx dedicates a whole slab page per slot size, so the 64k zone in
shared_dict.tis sixteen pages at 4k but only four at 16k — not enough to store a key and a value that fall into different size classes. The test failed on Apple Silicon while CI stayed green (#318).This adds a
macosjob to the NGINX workflow, mirroring the Linux one with a singlestable-1.30/staticconfiguration to keep the added time modest.perlandproveare already present on the runner image.Testing
Run locally on Apple Silicon (16k pages) against nginx 1.30.4, building all six example modules:
Files=5, Tests=28— all passt/shared_dict.t:Failed 4/14 subtestsld: symbol(s) not found for architecture arm64So the job does what it is meant to do: it fails on the bug #318 fixes, and passes once the fix is in.
One caveat
Across those runs,
t/shared_dict.tsubtest 3 ("check value 2") failed intermittently — roughly one run in four, independently of zone size or platform. The config usesworker_processes 2andcheck()retries until a response comes back from the expected worker, so this looks like a timing assumption in the test rather than anything in the module. It is not introduced by this PR, but wiring the suite into another job makes it more visible. I can look into it separately if you would like that sorted first.Checklist