Add hostname test #1376
Add hostname test #1376
Conversation
The test is ported from https://github.com/opencontainers/runtime-tools/blob/2802ff9ff5/validation/hostname/hostname.go The intent is to make sure that the hostname set in the spec is actually set within the container. Signed-off-by: chermehdi <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1376 +/- ##
=======================================
Coverage 68.80% 68.80%
=======================================
Files 119 119
Lines 12586 12664 +78
=======================================
+ Hits 8660 8714 +54
- Misses 3926 3950 +24 |
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
tests/rust-integration-tests/integration_test/src/tests/hostname/mod.rs
Outdated
Show resolved
Hide resolved
* Fix capitalisation in error messages. * Split tests into individual methods. Signed-off-by: chermehdi <[email protected]>
|
@chermehdi I have updated my comment, please take a look |
Signed-off-by: chermehdi <[email protected]>
|
Hey, mostly it is correct, but instead of removing the condition altogether, can you just return from the if-block? It will also indicate that we meant to return if no paths were given in the test instead of a potentially ambiguous the case of empty paths is not considered if someone has to update the that test later. Also, please amend the last commit instead of adding new commit for this 😃 |
|
That's what I had initially, but it's moot, since the loop below wouldn't run if the ro_paths is empty so no extra tests would run, the behavior is as expected with less code. about commits, I don't really put too much attention on amending commits as I assume we will squash all of them at the end of the day when merging, It just gives a better history line when looking at the PR for what changes happened and why/when. |
I can understand that the loop will not run if empty array, but we should have some sign that we have considered the empty array case, and the behavior is intentional. Also, we usually don't squash commits, but in this case, sure I'll squash this PR for merge |
Signed-off-by: chermehdi <[email protected]>
The test is ported from https://github.com/opencontainers/runtime-tools/blob/2802ff9ff5/validation/hostname/hostname.go
The intent is to make sure that the hostname set in the spec is actually set within the container.
Signed-off-by: chermehdi [email protected]