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

test: Notifiy more prominently & in more tests about false positives when running cargo test #6308

Merged
merged 2 commits into from
Dec 6, 2024

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Dec 4, 2024

This PR:

  • Moves the note about the false positive to the end of the test output, where it is more likely to be noticed
  • Also notes in test_modify_chat_disordered() and test_setup_contact_*(), in addition to the existing note in test_was_seen_recently()

…when running with `cargo test`

This:
- Moves the note about the false positive to the end of the test output,
  where it is more likely to be noticed
- Also notes in test_modify_chat_disordered() and
  test_setup_contact_*(), in addition to the existing note in
  test_was_seen_recently()
@Hocuri Hocuri changed the title test: Notifiy more prominently & in more tests about false positives when running with cargo test test: Notifiy more prominently & in more tests about false positives when running cargo test Dec 4, 2024
pub(crate) struct TimeShiftFalsePositiveNote;
impl Drop for TimeShiftFalsePositiveNote {
fn drop(&mut self) {
if std::thread::panicking() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It may not panic at this point but return Error in some tests also, but idk how to catch that. Anyway, the proposed approach is better than the previous assert! in the only test

@Hocuri Hocuri merged commit 39be591 into main Dec 6, 2024
37 checks passed
@Hocuri Hocuri deleted the hoc/notify-test-false-positives branch December 6, 2024 14:07
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.

2 participants