Skip to content

#1689: fix set -e masking test failures in Makefile#1785

Open
VasilevNStas wants to merge 2 commits into
zerocracy:masterfrom
VasilevNStas:1689-echo-masks
Open

#1689: fix set -e masking test failures in Makefile#1785
VasilevNStas wants to merge 2 commits into
zerocracy:masterfrom
VasilevNStas:1689-echo-masks

Conversation

@VasilevNStas

@VasilevNStas VasilevNStas commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Closes #1689

Fix set -e masking test failures in Makefile. The test and entry targets used echo "$$?" > target/*.exit after the command, but set -e causes the shell to exit before echo runs if the command fails (nonzero exit). Now uses || rc=$$? to capture the exit code without triggering the errexit trap.

Changes:

  • test target: save docker exit code via || rc=$$? pattern
  • entry target: same pattern for action test script

@VasilevNStas

Copy link
Copy Markdown
Contributor Author

@yegor256 plz review

Comment thread Makefile Outdated
img=$$(cat target/docker-image.txt)
docker run --rm --entrypoint '/bin/bash' "$${img}" -c 'bundle exec judges test --disable live --lib /action/lib /action/judges'
echo "$$?" > target/test.exit
img=$$(cat target/docker-image.txt); \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@VasilevNStas why do we need ; \? it's ONESHELL, should work without this

@VasilevNStas

Copy link
Copy Markdown
Contributor Author

Fixed — removed ; \ since Makefile uses .ONESHELL. Thanks for the note!

@VasilevNStas

VasilevNStas commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@yegor256

Both CI failures are pre-existing and unrelated to this PR:

  1. test_some_pull_hoc_size_handles_nil_additions_or_deletions — needs Jp.qoreset before calling Jp.qosearch. Fixed in PR #1667: guard Time.parse(ENV[TODAY]) against empty/invalid values #1819 (branch 1667-today-empty).

  2. NoMethodError: list_milestones in FakeOctokit — the milestone-was-set judge calls Fbe.octo.list_milestones which is not implemented in FakeOctokit. The patch was in branch 1724-nil-time-arithmetic (PR #1724 nil guard for time arithmetic in code-was-reviewed #1809) but that PR was closed due to mixed content. Needs a fresh PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Makefile verify target - set -e + echo $? makes test failures pass

2 participants