Add test coverage for parallel-build corruption, stale-expected check, base.mk fixes#31
Open
jnasbyupgrade wants to merge 3 commits into
Open
Conversation
…ion, stale-expected check, base.mk fixes): - Fix versioned SQL file generation race that doubled file content - Add check-stale-expected to catch orphaned test/expected/*.out files - Fix EXTRA_CLEAN target, add PGXN_REMOTE override, add base.mk include guard New BATS coverage in test/standard/: - versioned-sql-race.bats: races two concurrent `make all` invocations against the same versioned-SQL target across 5 iterations, asserting the file is never doubled (issue Postgres-Extensions#19) - check-stale-expected.bats: orphaned-file failure, the pg_regress alternate-output-file (_N.out) false-positive exemption plus a case proving it doesn't over-exempt, and the test/build/expected pair (issue Postgres-Extensions#14) - base-mk-fixes.bats: EXTRA_CLEAN targets the real test/results/ dir, PGXN_REMOTE default and override for tag/rmtag, and a double-include Makefile confirming base.mk's new guard is warning-free (issues Postgres-Extensions#7, #50, #53) Issue Postgres-Extensions#28 (variable rename) deliberately has no new test -- pure mechanical rename, no behavior change, already covered by the existing suite's DATA/build-output coverage. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…s a script, ordering fix, variable rename): - check-stale-expected logic moved to check-stale-expected.sh - Now depends on installcheck directly so it runs after pg_regress - EXTENSION_VERSION_FILES renamed to EXTENSION__CURRENT_VERSION__FILES - Replace the old "fails fast, before install/installcheck" test with one that verifies the new ordering via a make -n test dry-run (pg_regress's invocation must appear before check-stale-expected.sh), plus a test proving make test only fails on a stale file after pg_regress has produced real results (test/results/*.out) - Add pgxntool/check-stale-expected.sh to the dist file manifest Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…ipt move, file-type check, enable toggles) - check-stale-expected.sh moved to test/bin/check-stale-expected.sh - expected/ now also validated for non-*.out files (distinct error/exit code) - PGXNTOOL_ENABLE_CHECK_STALE_EXPECTED and PGXNTOOL_CHECK_EXPECTED_FILE_TYPES toggles added - Update dist-expected-files.txt for the new script path/directory - Add coverage for the non-.out file check (direct script invocation, since make's own exit status masks the child's real code) and for PGXNTOOL_CHECK_EXPECTED_FILE_TYPES=no - Fix the ordering test to exclude test-build's own pg_regress invocation when locating the real installcheck one, since the TEST_DEPS reordering in this commit changed their relative dry-run output order (no ordering relationship was ever guaranteed between them -- only the explicit check-stale-expected: installcheck edge matters) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.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.
Related pgxntool PR: Postgres-Extensions/pgxntool#61
New BATS coverage in
test/standard/:versioned-sql-race.bats(Fix results ordering, control file whitespace, ENABLE_* overrides #19): races two genuinely concurrentmake allinvocations against the same versioned-SQL target across 5 iterations, asserting the resulting file is never doubled. Verified by hand first that the old two-line recipe reproduces doubling (~1/3 of trials); the fixed single-redirect recipe never doubled across 15+ manual trials.check-stale-expected.bats(Make branch alignment check bidirectional #14): orphaned-file failure (both the standalone target and viamake testbefore PostgreSQL is needed), the pg_regress alternate-output-file (_N.out) false-positive exemption plus a case proving the exemption doesn't over-apply when the alternate's own base.sqlis also missing, and the same coverage for thetest/build/test/build/expectedpair.base-mk-fixes.bats(Add tests for test-install, test-build, verify-results #7, #50, #53):EXTRA_CLEANtargets the realtest/results/directory andmake cleanactually removes it;PGXN_REMOTEdefaults tooriginand is overridable for bothtagandrmtag; a throwaway Makefile that includespgxntool/base.mktwice (directly and via an intermediate module) parses with no overriding-recipe warnings.#28 (variable rename) deliberately has no new test — pure mechanical rename, no behavior change, already exercised by the existing suite's
DATA/build-output coverage.Full suite passes 228/228, 0 skips.