Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

[Exclusions] Missing fail is a fail #5518

Merged
merged 2 commits into from
Jun 25, 2021
Merged

[Exclusions] Missing fail is a fail #5518

merged 2 commits into from
Jun 25, 2021

Conversation

jeremyk-91
Copy link
Contributor

Goals (and why):
Not have too many errorprone exclusions. This one is very low risk - in general in a test if you have

try {
  bla();
} catch (ExceptionT e) {
  // expected
}

this will pass even if no exception is thrown. However, there are cases where not throwing is legit (see the suppression).

Implementation Description (bullets):

  • Add fail() where it should be
  • Add suppression

Testing (What was existing testing like? What have you done to improve it?):
Not much.

Concerns (what feedback would you like?):

  • Is the suppression correct?

Where should we start reviewing?: eh, it's small

Priority (whenever / two weeks / yesterday): whenever

Copy link
Contributor

@Jolyon-S Jolyon-S left a comment

Choose a reason for hiding this comment

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

4:1

@jeremyk-91 jeremyk-91 merged commit aefcdac into develop Jun 25, 2021
@delete-merged-branch delete-merged-branch bot deleted the jkong/missing-fail branch June 25, 2021 13:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants