Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(inpath): improve Windows and unix test for inpath() #4933

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JigyasuRajput
Copy link
Contributor

Description

Fixes #2928

This PR improves test coverage for the inpath() function by ensuring it is properly tested on both Windows and Unix-based systems. The function previously had limited coverage, particularly on Windows, where it was not correctly detecting executables in PATH.

Changes Made

Improvements to inpath() in cve_bin_tool/util.py:

  • Refactored Windows path handling to remove unnecessary nested operations, improving readability and efficiency.
  • Ensured consistent logic for checking if a binary exists in the system's PATH.

New and Improved Test Cases in test/test_util.py:

  1. test_inpath_windows

    • Mocks Windows PATH environment and verifies that inpath() correctly finds executables (python.exe).
    • Uses monkeypatching to simulate file presence and permissions in Windows directories.
  2. test_inpath_non_windows

    • Mocks Unix/Linux PATH and verifies that inpath() correctly detects executables with proper permissions (os.access).
  3. test_inpath_empty_path

    • Ensures inpath() returns False when the PATH environment variable is empty.

Impact

  • Increases test coverage for inpath() on both Windows and Unix-based systems.
  • Fixes potential issues in Windows detection by refining the logic for .exe handling.
  • Enhances reliability by covering edge cases such as empty PATH and incorrect file permissions.

How to Verify

Run the test suite to confirm functionality:

pytest test/test_util.py --tb=short -v

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

quick heads up: this one's showing as having a conflict, which is why the tests can't be run.

@JigyasuRajput
Copy link
Contributor Author

I've resolved the merge conflict. Thanks for pointing it out!

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

The tests aren't passing on windows, which looks very relevant to this:

 =========================== short test summary info ===========================
FAILED test/test_util.py::TestUtil::test_inpath_non_windows - AssertionError: assert False is True
 +  where False = inpath('python')
===== 1 failed, 2205 passed, 47 skipped, 7 warnings in 2223.54s (0:37:03) =====

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: improve test coverage for inpath() function
2 participants