-
Notifications
You must be signed in to change notification settings - Fork 359
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
Enable Whoops for most integration tests #3976
Conversation
EreMaijala
commented
Oct 1, 2024
- Also fixes an issue uncovered by this in status-full.phtml.
- Disables Whoops and logging for tests that expect failures.
- Also fixes an issue uncovered by this in status-full.phtml. - Disables Whoops and logging for tests that expect failures.
@@ -240,7 +240,8 @@ public function testSavedSearchSecurity(): void | |||
$this->findAndAssertLink($page, 'Log Out')->click(); | |||
|
|||
// Use user A's delete link, but try to execute it as user B: | |||
[$base, $params] = explode('?', $delete); | |||
[, $params] = explode('?', $delete); | |||
$session->setWhoopsDisabled(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$session->setWhoopsDisabled(true); | |
$session->setWhoopsDisabled(true); // We expect an error, so let's act like production mode for realistic testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @EreMaijala, this looks good to me!
I just thought it might be helpful to add a couple of comments, and I used that as an excuse to try the suggestion feature I mentioned yesterday. Apparently it's not new (at least four years old), but I only just learned about it. It's available as a button through the comment box when you add a comment to one or more lines of code while reviewing.
@@ -253,6 +254,7 @@ public function testSavedSearchSecurity(): void | |||
$this->findAndAssertLink($page, 'Log Out')->click(); | |||
|
|||
// Go back in as user A -- see if the saved search still exists. | |||
$session->setWhoopsDisabled(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$session->setWhoopsDisabled(false); | |
$session->setWhoopsDisabled(false); // go back to stricter error handling |
@demiankatz Thanks, and nice to see the suggestions in action. The lines were a bit too long, though, so I decided to add the comments above the actions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @EreMaijala!