Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bin: Fix exit code propagation #50

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Conversation

tchx84
Copy link
Contributor

@tchx84 tchx84 commented Apr 4, 2024

Now that we're loading entrypoints as modules, exit codes no longer propagate in the same way. Therefore, we must do that explicitly through the system exit function.

Without this the exit code is always ZERO and in practical terms all CI pipelines depending on this are always passing even when the tests are actually failing.

See 7825a0c.

To reproduce:

  1. Build latest jasmine-gjs from master (c6aa28b).

  2. Use the following sample test.

describe('test', function() {

    it('fails', function() {
        expect('one').toEqual('two');
    });
});
  1. Run it:
$ jasmine --no-config --verbose --no-color sample.js
$ echo $?

With out this PR the exit code is always ZERO.

Copy link
Owner

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Thanks for finding and fixing this! It's a pretty serious defect so I guess we should make a release right after it's merged.

bin/jasmine-runner.in Outdated Show resolved Hide resolved
bin/jasmine-runner.in Outdated Show resolved Hide resolved
bin/jasmine.in Outdated Show resolved Hide resolved
Now that we're loading entrypoints as modules, exit codes
no longer propagate  in the same way. Therefore, we  must
do that explicitly through the system exit function.

Without this the exit code is always  ZERO and in practical
terms all CI pipelines depending on this are always passing
even when the tests are actually failing.

See 7825a0c.
@tchx84
Copy link
Contributor Author

tchx84 commented Apr 6, 2024

@ptomato I addressed your comments, plus removed some unnecessary function wrapping...

@tchx84 tchx84 requested a review from ptomato April 6, 2024 18:19
Copy link
Owner

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Thanks!

@ptomato ptomato merged commit f6a42db into ptomato:master Apr 10, 2024
tchx84 added a commit to tchx84/Flatseal that referenced this pull request Apr 10, 2024
tchx84 added a commit to tchx84/Flatseal that referenced this pull request Apr 10, 2024
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.

2 participants