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

Create pull_request_template.md #162

Closed
wants to merge 1 commit into from

Conversation

Khushi-Dua
Copy link

#154

Update test_allFileMetadata.py
Here’s a concise analysis of the code:

  1. Hardcoded Path for Non-Existent File:

    • The test for a non-existent file uses a fixed path, which can lead to issues if the environment changes. Using a dynamic path (like tmp_path) ensures portability across platforms.
  2. Assertion Enhancements:

    • The test for an existing file asserts modification_time > 0, assuming a positive timestamp indicates a valid file. However, adding an isinstance check ensures the return type matches expectations, enhancing robustness.
  3. Error Handling Expectations:

    • The test_get_modification_time_non_existing_file expects -1 for non-existent files. This assumption should be confirmed by the AllFileMetadata class to ensure consistent handling of errors without throwing exceptions.
  4. Improved Descriptions:

    • Slightly refining docstrings would clarify the intent of each test, making the code easier to understand and maintain.

These adjustments will make the tests more reliable, readable, and adaptable to different environments.

@techy4shri
Copy link
Collaborator

Make the changes in the previous PR, do not create a new one unless you're unable to fix it.

@techy4shri techy4shri closed this Oct 29, 2024
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.

2 participants