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

Add exit code tests #275

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

joshbax189
Copy link
Contributor

Before changing the behavior of errors as discussed in #273 , I wanted to add more tests that check the current behavior.

This PR adds tests which check, for each command:

  • bad input exits with error
  • input with warnings exits with error when --strict is set
  • multiple inputs (i.e. commands which apply to multiple files or have multiple steps) should execute as far as possible
    when --allow-error is set, then exit with error

I'll add some script helpers that check error statuses and note any discrepancies I see.
I'll comment out and mark any currently failing expectations with #FIXME so that the CI run is clean.

@joshbax189
Copy link
Contributor Author

Right now, this only has checks for the analyze command, but I wanted to get feedback on the usage of the helper functions in test. Sorry if it triggers a lot of CI failures!

@jcs090218 jcs090218 added the CI label Nov 5, 2024
@jcs090218
Copy link
Member

I like the idea! These are my thoughts on the test implementation. :D

  • Open a new workflows/streams.yml and test it under the test/streams/ folder instead of testing for each command. Since most commands are implemented similarly (Node.js forward Emacs' streams), testing for each command could be more manageable.
  • From the point above, move all test-related files under test/streams/ (testing.sh, Eask-normal, Eask-warn, etc.)

WDYT? :)

@joshbax189
Copy link
Contributor Author

I think I chose the worst possible example command 💥 I want to test more than just Eask file errors and warnings, which I might have implied by this choice...

A better example might be eask compile. I want a normal .el file, an .el file that compiles with a warning and one that errors. I want to specifically test the "recover after error" behavior of the compile command itself, not just the warnings from the Eask file. E.g. test that eask compile --allow-error error.el normal.el should exit with non-zero status, but also compile normal.elc. (It does, by the way).

I'll add a few more examples in a new subfolder of test, like you suggested, then check in again later.

@jcs090218
Copy link
Member

No worries! I'm more keen to test this feature explicitly, so it's easier to track and test in one place. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants