Replace integration test review trigger with manual trigger

[mfisher87]

@JessicaS11 I think we should remove the pull_request_review trigger and logic. I don't know what we need to get this right, so I replaced it with a manual trigger. This will help us get PRs moving again. What do you think?

TODO:

• @JessicaS11 remove Integration Tests from workflows required to merge PRs

[github-actions]

👈 Launch a binder notebook on this branch for commit 4484b68

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit f551b3e

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.20%. Comparing base (a98409f) to head (f551b3e).
Report is 1 commits behind head on development.

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

HEAD has 1 upload less than BASE

Flag	BASE (a98409f)	HEAD (f551b3e)
	3	2

Additional details and impacted files

@@               Coverage Diff               @@
##           development     #595      +/-   ##
===============================================
- Coverage        71.67%   65.20%   -6.47%     
===============================================
  Files               37       37              
  Lines             3064     3064              
  Branches           596      596              
===============================================
- Hits              2196     1998     -198     
- Misses             765      981     +216     
+ Partials           103       85      -18     

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

[JessicaS11]

I don't know what we need to get this right, so I replaced it with a manual trigger.

I was looking at our setup, and I think the funny business might be coming from our if statement: if: "${{ !github.event.pull_request.head.repo.fork && (github.event.action != 'pull_request_review' || github.event.review.state == 'approved') }}"

My plain text of this logic is to run the test if (1) it's not a fork AND (2) it's (a) not a PR review OR (b) it's approved. I'm not sure why condition (a) is in there, since we're using the pull_request_review trigger but only want it to run if it's approved (and not on a fork). I know I don't have an intuition yet for saving computation by not evaluating past the first condition, but I can't make the logic of this one work out.

[JessicaS11]

@mfisher87 Any further thoughts on this?

[mfisher87]

Maybe we should close the PR? Things seem to be working well anyway right now. Can always come back to it :)

[JessicaS11]

Things seem to be working well anyway right now. Can always come back to it :)

Strong disagree. The Integration Tests are being triggered far more often than they should be.

[mfisher87]

When are they being triggered that you don't want them triggered? I've been struggling with them not being triggered when I do want them to be triggered. Do you think this manual triggering approach is the way forward for now?

[mfisher87]

We decided today to avoid the rabbit hole of GitHub Actions debugging and disable the integration test merge requirement and stop running integration tests on approval for now due to finnickiness of the current config (#603). I'm sure we can figure it out eventually :)