Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #347 +/- ##
==========================================
- Coverage 93.84% 93.83% -0.01%
==========================================
Files 105 105
Lines 18218 18214 -4
==========================================
- Hits 17096 17092 -4
Misses 1122 1122
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the EnvLayer.get_package_manager() unit test for Azure Linux 4 and RHEL 10 “unsupported” scenarios, aiming to address a test failure caused by stdout capture differences between PyCharm’s runner and CI.
Changes:
- Removed manual
sys.stdoutreassignment /StringIOcapture intest_get_package_manager_azure_linux_4_and_rhel10_not_supported. - Simplified assertions to only validate the empty-string return value from
get_package_manager()for unsupported distros.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for row in test_input_output_table: | ||
| self.envlayer.platform.linux_distribution = row[0] | ||
| distro.os_release_attr = row[1] | ||
|
|
||
| captured_output = io.StringIO() | ||
| sys.stdout = captured_output | ||
| result = self.envlayer.get_package_manager() | ||
| sys.stdout = sys.__stdout__ | ||
| self.assertEqual(row[2], captured_output.getvalue()) | ||
| self.assertEqual(result, "") | ||
| package_manager = self.envlayer.get_package_manager() | ||
| self.assertEqual(package_manager, "") |
kjohn-msft
added
bug
| captured_output = io.StringIO() | ||
| sys.stdout = captured_output | ||
| result = self.envlayer.get_package_manager() | ||
| sys.stdout = sys.__stdout__ |
There was a problem hiding this comment.
@yashnap, I think the problem was this line. See other examples in code where this is already implemented, for eg: test_reset_auto_assessment_log_file_if_needed_raise_exception()
you are setting sys.stdout to captured_output on line 171, but on line 174 while resetting sys.stdout it is set to "sys.__ stdout __ " (adding space because of github formatting). What does sys.__ stdout __ represent? Why not reset it to its original value?
Why it Failed in Pycharm:
The test failed in PyCharm because PyCharm uses a custom unittest runner that wraps and manages sys.stdout for output capture. This conflicts with manual reassignment of sys.stdout, causing it to be replaced at runtime with a _io.TextIOWrapper that does not support getvalue(), leading to an AttributeError.
Why it passed in PR (CI) ( My assumption/Copilot Suggestion)
The test passed in the PR pipeline because it was executed using the standard Python unittest runner (python -m unittest), which does not override or wrap sys.stdout. As a result, the manual reassignment sys.stdout = StringIO() remained intact, and the captured output could be read correctly using getvalue()
The failure is due to a conflict between manual sys.stdout reassignment and PyCharm’s test runner, which overrides stdout handling, while CI uses the standard unittest runner without such interference.
-Since we will be making changes to the PackageManager code to remove this behavior and actually return a package manager value, this can be used as temporary workaround