ci: update tests to run on pull requests

[straker]

No description provided.

[dbjorge]

We should discuss this as a standup/retro topic to make sure everyone's on the same page. There are pros and cons to both push and pull_request triggers, it's not a case where one is clearly better (@stephenmathieson and I were discussing this a few weeks ago):

1. push triggers as soon as a commit is pushed to the remote, where pull_request doesn't trigger until a PR is opened/pushed to. In practice, this means that if it takes you a few minutes to write a decent PR description, a push triggered test run will have already finished by the time you're submitting the PR (so you can see more quickly whether there are issues to fix), where a pull_request one won't be finished until you've context switched.
2. push runs against the commit as it was pushed to the repo. pull_request instead runs against a commit that is the result of GitHub simulating a merge of the PR into its target branch, which may not be the same as the PR branch's head commit if other PRs have been merged since creating the PR branch.
3. push will run against merge commits into develop, where pull_request will not. This is less necessary for pull_request because of point 2, but still a good idea; even with a pull_request trigger running tests against merge commits, it's still technically possible for 2 PRs (A and B) to have a logical merge conflict (eg, git doesn't detect a conflict, but a test added by A fails because of a change in B), and for a pull_request trigger to never catch it if B merges in between the last push to A and merging A.

I think the tradeoff between 1 and 2 leave me on the fence about using push vs pull_request for PR content - I'm happy to stick with whichever the team prefers. However, I think the possibility of 3 causing test failures to leak to production warrants always keeping a push trigger at least on develop/master/release branches, even if we use a pull_request trigger for PR branches.

[straker]

I know for certain we must have pull_request in order for our tests to run on 3rd party contributors (see #704). Should we do both push and pull_request?

[dbjorge]

Good point; at least for public repos where we need to handle contributions from forks, we should probably do at least pull_request + push for develop/master/release branches. We could just do pull_request + push (regardless of branch) for them since we don't pay for actions minutes in public repos, but it'd create a lot of redundant build work for not a lot of value imo.

Resolved