Fix failing integration tests

[chuckwondo]

Integration tests started failing after #792 was merged. This PR attempts to fix them.

Pull Request (PR) draft checklist - click to expand

• Please review our
  contributing documentation
  before getting started.
• Populate a descriptive title. For example, instead of "Updated README.md", use a
  title such as "Add testing details to the contributor section of the README".
  Example PRs: #763
• Populate the body of the pull request with:
  • A clear description of the change you are proposing.
  • Links to any issues resolved by this PR with text in the PR description, for
    example closes #1. See
    GitHub docs - Linking a pull request to an issue.
• Update CHANGELOG.md with details about your change in a section titled
  ## Unreleased. If such a section does not exist, please create one. Follow
  Common Changelog for your additions.
  Example PRs: #763
• Update the documentation and/or the README.md with details of changes to the
  earthaccess interface, if any. Consider new environment variables, function names,
  decorators, etc.

Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!

Pull Request (PR) merge checklist - click to expand

Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the @nsidc/earthaccess-support team in a comment and we
will help you out!

• Add unit tests for any new features.
• Apply formatting and linting autofixes. You can add a GitHub comment in this Pull
  Request containing "pre-commit.ci autofix" to automate this.
• Ensure all automated PR checks (seen at the bottom of the "conversation" tab) pass.
• Get at least one approving review.

---

📚 Documentation preview 📚: https://earthaccess--869.org.readthedocs.build/en/869/

[github-actions]

👈 Launch a binder notebook on this branch for commit cf77990

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit e6fd006

👈 Launch a binder notebook on this branch for commit bc73b03

👈 Launch a binder notebook on this branch for commit b3a1ebf

👈 Launch a binder notebook on this branch for commit e615b43

👈 Launch a binder notebook on this branch for commit cf73e1d

[chuckwondo]

@mfisher87, I closed #866 and opened this PR in hopes of getting past the integration test problem, but it is still happening. I have no idea why, by the integration test jobs are not using the code in the PR, thus the fix in the PR is not being run.

I can immediately tell this to be the case because this is at the top of the integration-tests step:

Run ./scripts/integration-test.sh
+ pytest tests/integration --cov=*** --cov=tests/integration --cov-report=term-missing --capture=no --tb=native --log-cli-level=INFO

However, in the PR I have removed the --cov=tests/integration option from that line, so it wouldn't show up if the code from my PR were being run instead.

I'm stumped at the moment as to why this is happening, particularly because I can similarly see that the unit tests are running the code from the PR.

I'm wondering if this has to do with the pull_request vs pull_request_target action, where perhaps pull_request pulls from the incoming code and pull_request_target pulls from the current code. This might also explain why the tests I'm attempting to fix did not initially fail before merging, because they were not actually executed until after merging.

cc: @jhkennedy

[chuckwondo]

Ah, I see that indeed the unit tests and integration tests are using different code.

Code for unit tests is pulled from the PR, as we want, but the event is for pull_request: https://github.com/nsidc/earthaccess/actions/runs/11659362104/job/32459819421?pr=792#step:2:55

Code for integration tests is pulled from main, not as we want, but the even is for pull_request_target: https://github.com/nsidc/earthaccess/actions/runs/11659361453/job/32470955273?pr=792#step:4:55

I think that for integration tests we should not be skipping the pull_request event, as I suspect that would be using the code from the PR, not from main.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 5.00000% with 19 lines in your changes missing coverage. Please review.

Project coverage is 58.72%. Comparing base (2f49c08) to head (cf73e1d).
Report is 70 commits behind head on main.

Files with missing lines	Patch %	Lines
earthaccess/store.py	0.00%	15 Missing ⚠️
earthaccess/api.py	20.00%	4 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (2f49c08) and HEAD (cf73e1d). Click for more details.

HEAD has 10 uploads less than BASE

Flag	BASE (2f49c08)	HEAD (cf73e1d)
	14	4

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #869       +/-   ##
===========================================
- Coverage   73.88%   58.72%   -15.17%     
===========================================
  Files          31       13       -18     
  Lines        2003     1100      -903     
===========================================
- Hits         1480      646      -834     
+ Misses        523      454       -69     

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

[jhkennedy]

@chuckwondo I'm approving this so we can merge and check the action.

I have not, however, reviewed the changes in earthaccess/api.py and earthaccess/store.py, or the integration tests themselves. I don't fully grok what's going on there, or why unit tests -> integration tests.

Unfortunately, I don't have time this week (or next) to dive into those, so feel free to request a review from someone else if you want someone to dive into those parts. I'm fine with just though.

[chuckwondo]

@chuckwondo I'm approving this so we can merge and check the action.

I have not, however, reviewed the changes in earthaccess/api.py and earthaccess/store.py, or the integration tests themselves. I don't fully grok what's going on there, or why unit tests -> integration tests.

Unfortunately, I don't have time this week (or next) to dive into those, so feel free to request a review from someone else if you want someone to dive into those parts. I'm fine with just though.

Thanks @jhkennedy! Fingers crossed.

Regarding the move of unit tests -> integration tests, that's because the unit tests added for testing the new pqdm_kwargs argument rely on logging in because calling earthaccess.download requires being authenticated, but unit tests do not make use of the EDL creds configured as secrets, only integration tests do. However, it's still unclear to me how the original changes passed the unit test workflows. The mysteries of github workflows continue to confound me in new ways.

[chuckwondo]

Ugh! Now the commit ref is fouled up. I'm going to attempt to fix this ASAP!

See https://github.com/nsidc/earthaccess/actions/runs/11693290544/job/32564527976