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

fix: add skip to vit #319

Merged
merged 2 commits into from
Jun 12, 2024
Merged

fix: add skip to vit #319

merged 2 commits into from
Jun 12, 2024

Conversation

jamescalam
Copy link
Member

@jamescalam jamescalam commented Jun 12, 2024

PR Type

Bug fix, Tests


Description

  • Added pytest.mark.skipif decorator to multiple test functions in test_vit.py to conditionally skip tests based on the RUN_HF_TESTS environment variable.
  • This change ensures that certain tests are only run when the RUN_HF_TESTS environment variable is set, preventing unnecessary test execution in environments where these tests are not required.

Changes walkthrough 📝

Relevant files
Tests
test_vit.py
Add conditional skip for ViT encoder tests based on environment
variable

tests/unit/encoders/test_vit.py

  • Added os import.
  • Added pytest.mark.skipif decorator to multiple test functions.
  • +16/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5]

    2

    🧪 Relevant tests

    Yes

    🔒 Security concerns

    No

    ⚡ Key issues to review

    None

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Simplify the test suite by consolidating repeated environment variable checks

    Replace the repeated checks for the environment variable "RUN_HF_TESTS" with a single
    check at the beginning of the test suite to simplify the code and avoid redundancy.

    tests/unit/encoders/test_vit.py [52-89]

    +# This decorator is moved to the top of the test suite
     @pytest.mark.skipif(
         os.environ.get("RUN_HF_TESTS") is None, reason="Set RUN_HF_TESTS=1 to run"
     )
     
    Suggestion importance[1-10]: 8

    Why: This suggestion effectively reduces redundancy and improves maintainability by consolidating repeated environment variable checks into a single check at the beginning of the test suite. This change simplifies the code and makes it easier to manage.

    8

    Copy link

    codecov bot commented Jun 12, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 77.01%. Comparing base (5120edc) to head (933778e).

    Current head 933778e differs from pull request most recent head c4685d8

    Please upload reports for the commit c4685d8 to get more accurate results.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main     #319      +/-   ##
    ==========================================
    - Coverage   77.56%   77.01%   -0.55%     
    ==========================================
      Files          45       45              
      Lines        2728     2728              
    ==========================================
    - Hits         2116     2101      -15     
    - Misses        612      627      +15     

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @jamescalam jamescalam merged commit 1916fec into main Jun 12, 2024
    6 checks passed
    @jamescalam jamescalam deleted the james/update-pinecone-test branch June 12, 2024 06:17
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    1 participant