CodyCBakerPhD commented 2 years ago

Trying to figure out why some of these tests that don't need to run on external forks are failing skip based on identical environment variable retrieval attempts

CodyCBakerPhD commented 2 years ago

And yes, as proven from #597 setting pull_request_target fixes the issue but as Julia said if it triggers automatically then our secrets could get stolen by anyone.

I'd rather just skip any of the tests that need secrets to run, especially when no code related to those functions is being changed.

Possible alternative could be a workflow_dispatch type logic for a manual trigger against the PR target?

codecov[bot] commented 2 years ago

Codecov Report

Merging #599 (72a56b0) into main (a3a5304) will decrease coverage by 0.77%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #599      +/-   ##
==========================================
- Coverage   88.34%   87.56%   -0.78%     
==========================================
  Files          59       59              
  Lines        3218     3218              
==========================================
- Hits         2843     2818      -25     
- Misses        375      400      +25

Flag	Coverage Δ
unittests	`87.56% <ø> (-0.78%)`	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/nwb_conversion_tools/tools/data_transfers.py	`56.33% <0.00%> (-35.22%)`	:arrow_down:

CodyCBakerPhD commented 2 years ago

OK that's all it took - I didn't realize the environment variable was being set, but being set to "". Fixed now.

catalystneuro / nwb-conversion-tools

testing CI from fork #599

Codecov Report