Fix kernel tainted test #3818
Fix kernel tainted test #3818
Conversation
I have only tested that it fails for unreleased 6.6.72 kernel but haven't tested the happy path and missed that it also failed because the types were different. Stupid me.
📝 WalkthroughWalkthroughThe pull request introduces a minor modification to two test files related to kernel taint testing. In both Changes
Sequence DiagramsequenceDiagram
participant Test as Test Function
participant Command as System Command
participant Assertion as Assertion Logic
Test->>Command: Execute 'cat /proc/sys/kernel/tainted'
Command-->>Test: Return output
Test->>Assertion: Process output
Assertion->>Assertion: Join output, remove newline
Assertion->>Assertion: Compare with "0"
The sequence diagram illustrates the updated test flow, showing how the output is now processed to remove newline characters before comparison. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/smoke_test/test_basic.py (1)
86-86: LGTM! Consider adding test cases for non-zero taint values.The fix correctly handles the output type from
shell.run_check()by joining the list before comparison. This resolves the type mismatch issue.Consider adding test cases that verify the error message when the kernel is tainted. This would help catch any future type-related issues early:
def test_kernel_tainted_error_message(shell, mocker): # Mock the shell.run_check to return a non-zero taint value mocker.patch.object(shell, 'run_check', return_value=['1']) with pytest.raises(AssertionError) as exc_info: test_kernel_not_tainted(shell) assert "Kernel tainted: ['1']" in str(exc_info.value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/smoke_test/test_basic.py(1 hunks)tests/supervisor_test/test_supervisor.py(1 hunks)
🔇 Additional comments (2)
tests/supervisor_test/test_supervisor.py (2)
200-200: LGTM! Consistent handling of shell output across test files.The change aligns with the fix in
test_basic.py, ensuring consistent handling of shell output in both test files.
Line range hint
86-86: Verify the fix works with different kernel versions.Let's confirm that
/proc/sys/kernel/taintedoutput format is consistent across kernel versions.Also applies to: 200-200
✅ Verification successful
The fix correctly handles kernel taint file output format
The implementation in
test_kernel_not_taintedis robust and will work reliably across different kernel versions because:
- It properly handles both single-line and multi-line output formats
- The kernel taint file format is standardized to output a single number
- The string comparison with "0" works correctly regardless of trailing newlines
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check kernel taint file output format across different kernel versions # Note: This helps verify that the fix will work reliably # Check current kernel version uname -r # Check taint file format cat /proc/sys/kernel/tainted # Check if output contains only digits and newline if ! grep -qE '^[0-9]+$' /proc/sys/kernel/tainted; then echo "Unexpected format in taint file" exit 1 fiLength of output: 291
Script:
#!/bin/bash # Check the test file implementation and mocks cat tests/supervisor_test/test_supervisor.pyLength of output: 8350
| @@ -83,4 +83,4 @@ def test_kernel_not_tainted(shell): | |||
| """Check if the kernel is not tainted - do it at the end of the | |||
| test suite to increase the chance of catching issues.""" | |||
| output = shell.run_check("cat /proc/sys/kernel/tainted") | |||
| assert output == "0\n", f"Kernel tainted: {output}" | |||
| assert "\n".join(output) == "0", f"Kernel tainted: {output}" | |||
There was a problem hiding this comment.
Isn't that fixing it on both sides? 🤔
How about:
| assert "\n".join(output) == "0", f"Kernel tainted: {output}" | |
| assert output.strip() == "0", f"Kernel tainted: {output}" |
There was a problem hiding this comment.
That won't work, the thing is the return type is a list of strings. This time I actually checked the test doesn't fail 🙈
There was a problem hiding this comment.
Hm, I see, and "\n".join(output) won't add a newline if there is only one element. Yeah make sense.
I have only tested that it fails for unreleased 6.6.72 kernel but haven't tested the happy path and missed that it also failed because the types were different. Stupid me.
Summary by CodeRabbit
test_kernel_not_taintedfunction assertions in multiple test files to handle newline characters more precisely when checking kernel tainted status.