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

WIP: Console break on error in batch mode #1692

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gramian
Copy link
Collaborator

@gramian gramian commented Aug 17, 2024

What does this PR do?

Overall, this change alters the console behavior such that if an error occurs in batch mode or during a script, execution is halted and if in batch mode, the console is exited.

In particular these changes are introduced in Console.java:

  1. Make parse method have 3 arguments (see line 731). The third argument is a boolean indicating batch mode, which is true by default.
  2. Add wrapper method with 2 arguments to minimize changes (see line 727)
  3. New parse does not consume exeception anymore, but throws them upwards (see line 742ff).
  4. "Lower" execute method now throws also RuntimeExeception (see line line 223)
  5. "Upper" execute method uses new parse method depending on if batch mode is set.

Motivation

Currently console continues to execute commands in scripts or batch even though previous command fails. This can produce unwanted results. Especially tracking errors is complicated since if the last line passes, it seems as if a script or batch is successful.

Additional Notes

@lvca

  • ConsoleTest: testNotNullProperties fails, is this test faulty?
  • Need help constructing tests for this.
  • The "upper" execute method (line 155) should be renamed so there are not two methods names execute.
  • Are the exeception-related changes OK?

Checklist

  • I have run the build using mvn clean package command
  • My unit tests cover both failure and success scenarios

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.

1 participant