icepyx icon indicating copy to clipboard operation
icepyx copied to clipboard

Reinstate test_download_granules_without_subsetting

Open weiji14 opened this issue 1 year ago • 2 comments

Trying to increase code coverage of ipx.Query class's order_granules and download_granules methods, by ensuring that granules can be ordered from NSIDC and downloaded with the subset=False option.

TODO:

  • [x] Add test which should fail due to not capturing stdout and stderr messages
  • [x] Regex match on stdout and stderr so test will pass
  • [x] Check that downloaded files are actually not subsetted (kinda)

Adjacent stuff done in this PR

  • [x] Fix a possible race condition in case the NSIDC order status if completed right at the start (e.g. when order was cached/done already).
  • [x] Upload code coverage for 'behind Earthdata' tests via Travis CI

weiji14 avatar Aug 22 '24 16:08 weiji14

Binder :point_left: Launch a binder notebook on this branch for commit 115ce4ce8f3223385523148f8e5e0753f26a717d

I will automatically update this comment whenever this PR is modified

Binder :point_left: Launch a binder notebook on this branch for commit 9090378f932bec131ce5b213276ed006249f1301

Binder :point_left: Launch a binder notebook on this branch for commit 58a4a68f2b7581f5f9e4c1454ea1948d00f1243a

Binder :point_left: Launch a binder notebook on this branch for commit 97ba3de3d54d76f4a74c0bf70209bcbfac6f4236

Binder :point_left: Launch a binder notebook on this branch for commit 29a7230289ef6973ce3d902349f14e75bd748044

Binder :point_left: Launch a binder notebook on this branch for commit e47a1a54af6dd3bc52f18b8721a619be3d251349

Binder :point_left: Launch a binder notebook on this branch for commit 4fbbb98b4824510aba36d49ad6ec959396cc58cf

Binder :point_left: Launch a binder notebook on this branch for commit 358a26510b50fefbbb3a63096428ac243fd84d07

Binder :point_left: Launch a binder notebook on this branch for commit 1e8799ceb245285efed43047883443de82653ef3

Binder :point_left: Launch a binder notebook on this branch for commit d3a0b8a39719ce2519c9a686348932cf209b78aa

Binder :point_left: Launch a binder notebook on this branch for commit 65328a60f7e034097909f8fe94d9e0a6fef9d2fe

Binder :point_left: Launch a binder notebook on this branch for commit 6434fae148715f923942b826675add06f43b60ea

github-actions[bot] avatar Aug 22 '24 16:08 github-actions[bot]

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 65.17%. Comparing base (572e48c) to head (6434fae). Report is 22 commits behind head on development.

Files with missing lines Patch % Lines
icepyx/tests/test_behind_NSIDC_API_login.py 0.00% 15 Missing :warning:
icepyx/core/granules.py 0.00% 3 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (572e48c) and HEAD (6434fae). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (572e48c) HEAD (6434fae)
3 2
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #581      +/-   ##
===============================================
- Coverage        71.08%   65.17%   -5.91%     
===============================================
  Files               37       37              
  Lines             3043     3061      +18     
  Branches           594      596       +2     
===============================================
- Hits              2163     1995     -168     
- Misses             769      981     +212     
+ Partials           111       85      -26     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 22 '24 17:08 codecov[bot]

Got the test_download_granules_without_subsetting test to pass when I triggered it at https://github.com/icesat2py/icepyx/actions/runs/10631387124/job/29472168206#step:4:25.

weiji14 avatar Aug 30 '24 10:08 weiji14

Got the test_download_granules_without_subsetting test to pass when I triggered it

Out of curiousity, how did you trigger it? It doesn't have a manual trigger, and should only run with an "approved" PR review status (but we're having some issues with this, clearly, so trying to learn how you were able to make it go).

JessicaS11 avatar Aug 30 '24 19:08 JessicaS11

Got the test_download_granules_without_subsetting test to pass when I triggered it

Out of curiousity, how did you trigger it? It doesn't have a manual trigger, and should only run with an "approved" PR review status (but we're having some issues with this, clearly, so trying to learn how you were able to make it go).

I triggered it at commit d3a0b8a39719ce2519c9a686348932cf209b78aa by modifying .github/workflows/integration_test.yml to run on all branches for pull requests (not just those that target the main branch). Downside with this trick is that you'll need to have a commit to revert the changes later.

weiji14 avatar Sep 02 '24 22:09 weiji14

Got the test_download_granules_without_subsetting test to pass when I triggered it

Out of curiousity, how did you trigger it? It doesn't have a manual trigger, and should only run with an "approved" PR review status (but we're having some issues with this, clearly, so trying to learn how you were able to make it go).

I triggered it at commit d3a0b8a by modifying .github/workflows/integration_test.yml to run on all branches for pull requests (not just those that target the main branch). Downside with this trick is that you'll need to have a commit to revert the changes later.

Sneaky! Would that work from a fork too?

JessicaS11 avatar Sep 03 '24 20:09 JessicaS11

Sneaky! Would that work from a fork too?

Not sure :sweat_smile: I had a look at the docs and couldn't figure it out. Understandably, this sounds like a security concern, but secrets shouldn't be accesible from forks, and there should be an 'Approve to run' button that is set for new contributors according to https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/approving-workflow-runs-from-public-forks

weiji14 avatar Sep 03 '24 21:09 weiji14