Skip to content

fix: make test actually fails when tests fail instead of always exiting 0 - #88

Merged
Senthil455 merged 1 commit into
Senthil455:mainfrom
DivyashreeR008:fix/make-test-exit-code
Jun 17, 2026
Merged

fix: make test actually fails when tests fail instead of always exiting 0#88
Senthil455 merged 1 commit into
Senthil455:mainfrom
DivyashreeR008:fix/make-test-exit-code

Conversation

@DivyashreeR008

Copy link
Copy Markdown
Collaborator

Closes #53

What was wrong

Every test command in the \make test\ target used || true, so the target always exited with code 0. This meant:

  • CI pipelines always showed green regardless of test failures
  • Developers running make test locally saw red output but the command still succeeded
  • Regressions could be introduced and deployed without detection

What changed

The fix replaces || true\ with a shell variable that tracks failures across all services:

\\makefile
@failed=0;
cd services/api-gateway-node && npm test || failed=1;
...
if [ failed -ne 0 ]; then exit 1; fi;
\\

This runs every service's test suite regardless of individual failures, then exits with code 1 if any test failed. No more silent swallowing of failures.

Why not just remove || true

Simply dropping || true\ would make make abort at the first failing service and skip the rest. The chosen approach runs all tests and reports the overall result, which is more useful for developers iterating on multiple services.

…ng 0

The make test target used || true on every test command, so it always
exited with code 0 even when tests failed. This gave false green status
in CI pipelines and hid regressions.

Instead of blindly removing || true (which would make make abort at the
first failure and skip remaining services), this change captures each
test exit code in a variable so all services run regardless. If any
test fails, the target exits with code 1 after all have finished.
@DivyashreeR008

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough review! Glad the double-escaped dollar sign checked out. No changes needed — merging as-is.

@Senthil455
Senthil455 merged commit 5aaa075 into Senthil455:main Jun 17, 2026
6 of 19 checks passed

@Senthil455 Senthil455 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

gh: Not Found (HTTP 404)
{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}

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.

HIGH: Make test silently swallows all failures with || true - always exits 0

2 participants