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

Retry ActionableErrors when running tests #50941

Merged
merged 1 commit into from Apr 4, 2024

Conversation

andrewn617
Copy link
Contributor

@andrewn617 andrewn617 commented Feb 1, 2024

Motivation / Background

This Pull Request has been created because I want to make it easier to retry actionable errors that occur when running tests. Rails already knows how to recover these errors, so lets make it possible for the user to recover from them and run their tests without exiting the process.

Detail

Allow Actionable Errors encountered when running tests to be retried.

Migrations are pending. To resolve this issue, run:

        bin/rails db:migrate

You have 1 pending migration:

db/migrate/20240201213806_add_a_to_b.rb
Run pending migrations? [Yn] Y
== 20240201213806 AddAToB: migrating =========================================
== 20240201213806 AddAToB: migrated (0.0000s) ================================

Running 7 tests in a single process (parallelization threshold is 50)
Run options: --seed 22200

# Running:

.......

Finished in 0.243394s, 28.7600 runs/s, 45.1942 assertions/s.
7 runs, 11 assertions, 0 failures, 0 errors, 0 skips

This feature is only available in an interactive terminal.

Additional information

Right now this is only implemented in the Test Runner, but I'd also like to follow up with a similar implementation for Rails::Command::CorrectableNameErrors.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@andrewn617 andrewn617 force-pushed the actionable-cli branch 3 times, most recently from 085ef36 to b481c34 Compare February 1, 2024 22:11
@rails-bot rails-bot bot added the docs label Feb 1, 2024
@gmcgibbon
Copy link
Member

Looks like we have some railties tests continuously timing out. We'll look into that ASAP.

@andrewn617 andrewn617 changed the title Retry ActionErrors when running tests Retry ActionableErrors when running tests Feb 2, 2024
@andrewn617
Copy link
Contributor Author

Looks like we have some railties tests continuously timing out. We'll look into that ASAP.

Needed to turn off the flag in the generated app in a few more tests 👍

@gmcgibbon
Copy link
Member

gmcgibbon commented Feb 15, 2024

@andrewn617 Let's go ahead and rebase + add the new framework default, so new apps can onboard to this behaviour.

@andrewn617 andrewn617 force-pushed the actionable-cli branch 2 times, most recently from fddb9cd to 7694426 Compare February 15, 2024 19:23
@andrewn617 andrewn617 force-pushed the actionable-cli branch 2 times, most recently from 501627b to c86c1d5 Compare March 21, 2024 20:57
@andrewn617 andrewn617 force-pushed the actionable-cli branch 2 times, most recently from 12ac53a to 3898474 Compare April 2, 2024 18:45
@andrewn617
Copy link
Contributor Author

Ultimately @gmcgibbon and I decided against a framework configuration for this. The other use case we are working on is for "Did you mean" errors when the command name is typoed. In that case, the app is not even loaded yet, so the configuration is not available to be checked. So instead, we are going with STDOUT.tty? for determining if the console is interactive.

Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

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

LGTM, @andrewn617 can you please update the description of the PR and git commit message so this is accurate to the new approach. I'll merge after that.

If running in an interactive console, the user will be given the option to correct the error and re-run the tests.

Co-authored-by: Gannon McGibbon <gannon.mcgibbon@gmail.com>
@andrewn617
Copy link
Contributor Author

@gmcgibbon Done.

@gmcgibbon gmcgibbon merged commit d4c40b6 into rails:main Apr 4, 2024
4 checks passed
@gmcgibbon
Copy link
Member

Thanks! ❤️

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

Successfully merging this pull request may close these issues.

None yet

2 participants