- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
          ✅  [Maintenance] Fixes api-server/tests/unit/pact_broker testing
          #8473
        
          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
Conversation
api-server/tests/unit/pact_broker testing
      
          Codecov Report✅ All modified and coverable lines are covered by tests. 
 Additional details and impacted files@@             Coverage Diff             @@
##           master    #8473       +/-   ##
===========================================
- Coverage   87.64%   69.05%   -18.60%     
===========================================
  Files        1983      889     -1094     
  Lines       77279    39739    -37540     
  Branches     1333      175     -1158     
===========================================
- Hits        67729    27440    -40289     
- Misses       9151    12242     +3091     
+ Partials      399       57      -342     
 
 Continue to review full report in Codecov by Sentry. 
 🚀 New features to boost your workflow:
  | 
    
8a8c575    to
    0810667      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes failing pact broker tests in the API server by correcting test fixtures, improving skip mechanisms, and enhancing documentation. The tests were failing due to incorrect attribute references and dependency override issues.
Key changes:
- Fixed test fixtures and dependency overrides that were causing AttributeError failures
 - Replaced 
pytest.failwithpytest.skipfor better handling of missing credentials - Renamed test functions for clarity and improved documentation
 
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description | 
|---|---|
| test_pact_licensed_items.py | Removed unused import and improved test naming | 
| test_pact_checkout_release.py | Fixed dependency overrides and improved fixture cleanup | 
| conftest.py | Enhanced credential handling and dependency override management | 
| README.md | Improved documentation with clearer instructions | 
| Makefile | Streamlined test command documentation | 
| tests/unit/conftest.py | Added type annotations to improve code clarity | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
        
          
                services/api-server/tests/unit/pact_broker/test_pact_checkout_release.py
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                services/api-server/tests/unit/pact_broker/test_pact_checkout_release.py
          
            Show resolved
            Hide resolved
        
      …release.py Co-authored-by: Copilot <[email protected]>
        
          
                services/api-server/tests/unit/pact_broker/test_pact_checkout_release.py
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements
          
 | 
    



What do these changes do?
skiptest mechanismIn detail (AI)
This pull request refactors and improves the pact broker test setup for the API server, focusing on clearer test logic, more robust fixture management, and better handling of credentials and dependencies. The changes streamline how dependencies are overridden, improve the handling of missing credentials, and update test naming for clarity.
Test infrastructure improvements:
pact_broker_credentialsfixture to usepytest.skipinstead ofpytest.failwhen credentials are missing, and clarified its return type. This ensures tests are skipped gracefully if required environment variables or CLI arguments are not provided.running_test_server_urlfixture to use an inline function for mocking identity, and ensured cleanup by removing the override after the test completes. [1] [2]Test logic and naming:
test_provider_against_pactto more descriptive names (test_osparc_api_server_checkout_release_pactandtest_osparc_api_server_licensed_items_pact), and removed unnecessary environment variable checks for test skipping, relying instead on the improved fixture logic. [1] [2]Fixture and import clean-up:
webserver_rpcandresource_usage_tracker, using more robust patching and dependency overrides, and cleaning up imports for clarity and correctness. [1] [2]Documentation updates:
README.mdinstructions for pact broker usage, adding clearer steps for installing and publishing contracts.Related issue/s
How to test
Dev-ops
None