Skip to content

Commit

Permalink
test: Don't run same test multiple times
Browse files Browse the repository at this point in the history
Co-authored-by: Alan Szepieniec <[email protected]>
  • Loading branch information
Sword-Smith and aszepieniec committed Jan 24, 2025
1 parent 7772861 commit 3687558
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/models/blockchain/type_scripts/native_currency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1652,7 +1652,7 @@ pub mod test {
);
}

#[proptest]
#[proptest(cases = 1)]
fn assertion_failure_is_caught_gracefully() {
// This test is supposed to catch wrong compilation flags causing
// causing asserts not to be caught by catch_unwind.
Expand Down

4 comments on commit 3687558

@jan-ferdinand
Copy link
Member

Choose a reason for hiding this comment

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

Why is this a proptest? Within the panic::catch_unwind(…), why assert!(false, …) instead of panic!(…)? 🤔

@jan-ferdinand
Copy link
Member

@jan-ferdinand jan-ferdinand commented on 3687558 Jan 24, 2025

Choose a reason for hiding this comment

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

Suggestion:

/// Catch wrong compilation flags.
#[test]
fn assertion_failure_is_caught_gracefully() {
    assert!(panic::catch_unwind(|| panic!()).is_err());
}

@aszepieniec
Copy link
Contributor

Choose a reason for hiding this comment

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

With this suggestion you are not verifying that the proptest framework is not swallowing asserts.

@jan-ferdinand
Copy link
Member

Choose a reason for hiding this comment

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

Ah. Neither comment nor test name suggest that this test is about proptest, which explains (part of) my confusion.

Please sign in to comment.