Skip to content

MAIN: Add test for connect timeout#596

Merged
Enet4 merged 2 commits intoEnet4:masterfrom
naterichman:connect-timeout-test
Nov 18, 2024
Merged

MAIN: Add test for connect timeout#596
Enet4 merged 2 commits intoEnet4:masterfrom
naterichman:connect-timeout-test

Conversation

@naterichman
Copy link
Copy Markdown
Contributor

No description provided.

@Enet4 Enet4 added chore For keeping the project tidy A-lib Area: library C-ul Crate: dicom-ul labels Nov 7, 2024
@Enet4 Enet4 self-requested a review November 7, 2024 16:00
Copy link
Copy Markdown
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

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

The extra timeout tolerance was too short for my machine, which would often exceed by a few milliseconds. I made a few tweaks to clarify when this happens and increase the tolerance from 10ms to 25ms. I can push them to this branch soon, if that's okay with you.

ul/tests/association.rs --- 1/3 --- Rust
2    use dicom_ul::ClientAssociationOptions;
3    use rstest::rstest;
4    use std::time::Instant;
   6 const TIMEOUT_TOLERANCE: u64 = 25;
   7 
   8 #[rstest]
   9 #[case(100)]
   10 #[case(500)]

ul/tests/association.rs --- 2/3 --- Rust
16    
17        let now = Instant::now();
18        let _res = scu_init.establish_with("RANDOM@167.167.167.167:11111");
19        let elapsed = now.elapsed();
20        assert!(elapsed.as_millis() < (timeout + 10).into());
   22     assert!(
   23         elapsed.as_millis() < (timeout + TIMEOUT_TOLERANCE).into(),
   24         "Elapsed time {}ms exceeded the timeout {}", elapsed.as_millis(), timeout
   25     );
   26 }
   27 
   28 #[cfg(feature = "async")]

ul/tests/association.rs --- 3/3 --- Rust
38            .await;
39        assert!(res.is_err());
40        let elapsed = now.elapsed();
41        println!("Elapsed time: {:?}", elapsed);
42        assert!(elapsed.as_millis() < (timeout + 10).into());
   47     assert!(
   48         elapsed.as_millis() < (timeout + TIMEOUT_TOLERANCE).into(),
   49         "Elapsed time {}ms exceeded the timeout {}ms", elapsed.as_millis(), timeout
   50     );
   51 }

- the connection timeout limit may be overshot on some machines (windows)
- also improve assertion message
Copy link
Copy Markdown
Owner

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

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

Should be OK now. 👍 Thanks!

@Enet4 Enet4 merged commit e81a438 into Enet4:master Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-lib Area: library C-ul Crate: dicom-ul chore For keeping the project tidy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants