Skip to content

Fix integration tests for HuggingFace authentication Issue #187#260

Closed
mrgear111 wants to merge 2 commits intoopenclimatefix:mainfrom
mrgear111:main
Closed

Fix integration tests for HuggingFace authentication Issue #187#260
mrgear111 wants to merge 2 commits intoopenclimatefix:mainfrom
mrgear111:main

Conversation

@mrgear111
Copy link
Copy Markdown

This PR addresses the issue where tests fail when running on the OCF GitHub due to missing HuggingFace authentication. The changes:

  1. Add proper skipif decorators to integration tests that require HF authentication
  2. Create a mock module for PV data testing
  3. Ensure integration tests are properly marked and can be skipped by default
  4. Fixes Make sure all tests work  #187

How Has This Been Tested?
[x] Yes
I've tested this by:
Running tests without the --run-integration flag to verify all tests pass or are properly skipped
Running tests with --run-integration to verify integration tests are attempted but skipped when HF_TOKEN is not available
Verifying that unit tests with mocked data pass correctly

Checklist:
[x] My code follows OCF's coding style guidelines
[x] I have performed a self-review of my own code
[x] I have made corresponding changes to the documentation
[x] I have added tests that prove my fix is effective or that my feature works
[x] I have checked my code and corrected any misspellings

@mrgear111
Copy link
Copy Markdown
Author

@Sukh-P Hello sir I have tried to solve the issue.. please review it!

@mrgear111
Copy link
Copy Markdown
Author

@peterdudfield Please review this PR

Moved test utility functions from quartz_solar_forecast/eval/test_pv.py to
tests/utils/hf_data_utils.py and updated all import references. This keeps
test-related code in the test directory structure as suggested in PR review.
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.98%. Comparing base (4819c9d) to head (1f74d25).
⚠️ Report is 68 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #260      +/-   ##
==========================================
- Coverage   68.29%   58.98%   -9.31%     
==========================================
  Files          27       27              
  Lines        1085     1085              
==========================================
- Hits          741      640     -101     
- Misses        344      445     +101     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@peterdudfield
Copy link
Copy Markdown
Contributor

I can see that there are over 5000 files added, This makes the PR very hard to review? Could you remove some of these if not needed?

@hrsvrn
Copy link
Copy Markdown
Contributor

hrsvrn commented Mar 20, 2025

@peterdudfield i think you should close this issue as i think he committed all his virtual environment files which resulted in 5000+ files and 5 million lines of PR

@mrgear111 mrgear111 closed this Oct 9, 2025
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.

Make sure all tests work

3 participants