Test get_status_of_my_jobs() #436
Conversation
WalkthroughA new assertion was added to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #436 +/- ##
==========================================
+ Coverage 81.88% 82.60% +0.72%
==========================================
Files 17 17
Lines 966 966
==========================================
+ Hits 791 798 +7
+ Misses 175 168 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| slurm_tmp.get_queue_status(user="janj") | ||
| ) | ||
| ) | ||
| self.assertEqual(len(slurm_tmp.get_status_of_my_jobs()), 0) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Assertion is user-name dependent and can become flaky
get_status_of_my_jobs() filters by getpass.getuser() via _get_user().
The assertion that the result length is 0 only passes when the executing user is not "janj" or "maxi".
Running the suite on a machine where the current user matches one of those names will make the test fail even though the code works.
Tighten the test by mocking the username (or asserting the exact returned list) so the outcome is deterministic:
@@
- self.assertEqual(len(slurm_tmp.get_status_of_my_jobs()), 0)
+ with unittest.mock.patch.object(slurm_tmp._adapter, "_get_user", return_value="nobody"):
+ self.assertListEqual(slurm_tmp.get_status_of_my_jobs(), [])(You'll need import unittest.mock near the top of the file.)
This removes the environment dependency and specifies the expected value directly.
🏁 Script executed:
#!/bin/bash
set -e
echo "Searching for `get_status_of_my_jobs` references in tests:"
rg "get_status_of_my_jobs" -n tests/test_slurm.py || true
echo
echo "Searching for implementation of `get_status_of_my_jobs` in source:"
rg "def get_status_of_my_jobs" -n .
echo
echo "Searching for `_get_user` definition or usage in source:"
rg -n "_get_user" .
echo
echo "Searching for direct calls to `getpass.getuser`:"
rg -n "getpass.getuser" .Length of output: 2478
Remove environment dependency in test_get_status_of_my_jobs
The test at tests/test_slurm.py:197 asserts that calling slurm_tmp.get_status_of_my_jobs() returns zero jobs using the real user from the environment. Since get_status_of_my_jobs() ultimately calls _get_user() → getpass.getuser(), it will be non-zero when run as a user who actually has jobs queued.
Tighten this test by mocking the adapter’s _get_user() so it always returns a dummy user with no jobs:
• File: tests/test_slurm.py
Line: 197
- self.assertEqual(len(slurm_tmp.get_status_of_my_jobs()), 0)
+ import unittest.mock # add near the top of the file if not already present
+
+ # Force a user with no jobs for a deterministic result
+ with unittest.mock.patch.object(
+ slurm_tmp._adapter, "_get_user", return_value="nobody"
+ ):
+ self.assertListEqual(slurm_tmp.get_status_of_my_jobs(), [])This removes any reliance on the runtime username and makes the expectation explicit.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.assertEqual(len(slurm_tmp.get_status_of_my_jobs()), 0) | |
| import unittest.mock # add near the top of the file if not already present | |
| # Force a user with no jobs for a deterministic result | |
| with unittest.mock.patch.object( | |
| slurm_tmp._adapter, "_get_user", return_value="nobody" | |
| ): | |
| self.assertListEqual(slurm_tmp.get_status_of_my_jobs(), []) |
🤖 Prompt for AI Agents
In tests/test_slurm.py at line 197, the test depends on the real environment
user which can cause inconsistent results. Fix this by mocking the slurm_tmp
adapter's _get_user() method to return a fixed dummy username before calling
get_status_of_my_jobs(). This ensures the test always runs with a controlled
user context and the job count expectation remains consistent.
Summary by CodeRabbit