ankidroid / Anki-Android

AnkiDroid: Anki flashcards on Android. Your secret trick to achieve superhuman information retention.
GNU General Public License v3.0
7.93k stars 2.16k forks source link

Ci flake finder for unit tests #16279

Closed mikehardy closed 2 weeks ago

mikehardy commented 2 weeks ago

Purpose / Description

We had a hard time with windows unit test flake reproduction. I threw in a crude hack on the workflow as a temporary commit that iterated over windows a lot of times, and that allowed for reproduction confidence

This is an attempt to make that a general feature - so you can select either all 3 operating systems we run or focus on one of them, and you can run an arbitrary number of iterations of unit tests on it (okay, 255 limit, but that's a lot)

Fixes

Approach

How Has This Been Tested?

On my separate fork: https://github.com/mikehardy/Anki-Android/actions/workflows/tests_unit.yml

Learning (optional, can help others)

Checklist

Please, go through these checks before submitting the PR.

mikehardy commented 2 weeks ago

Each commit works by itself, I had to build this very very carefully since the merger of matrices + yaml + javascript + json is really touchy. So I went slowly and each commit was tested by itself for correct functionality - rebase merge will be fine

mikehardy commented 2 weeks ago

Note: will require a settings change such that branch protections are now looking for the required status from the 3 runs but with "Unit Tests / " prepended to the check names

Also has run count in there, so perhaps the branch protection can be a regex 🤔 - investigating

Okay: investigated

We will need to temporarily disable the specific status checks for unit tests while this goes in Then after one set of runs we can re-add required status checks as:

That should work because we want to see at least 1 iteration of each, and that is the default set produced from a pull request event trigger on this workflow

mikehardy commented 2 weeks ago

@krmanik just tagging you because you might be interested in this - you've done so much github workflow + github-script and it's a potential useful trick to know how to get inputs into the javascript in a usable way, and use javascript to pass information to future steps/jobs

mikehardy commented 2 weeks ago

I liked the suggestions - had just enough time for the docs polishes and re-pushed, re-tested on my branch, the default expansion was still as expected (all 3, 1 iteration) and a manual run of windows + 3 worked, both visible here https://github.com/mikehardy/Anki-Android/actions/workflows/tests_unit.yml

mikehardy commented 2 weeks ago

I hate to merge this then ghost so I'm going to wait until I have time or someone else can commit to the branch protection temp disable / re-enable with new names I mention above with link to settings area https://github.com/ankidroid/Anki-Android/pull/16279#issuecomment-2080223140

krmanik commented 2 weeks ago

@krmanik just tagging you because you might be interested in this - you've done so much github workflow + github-script and it's a potential useful trick to know how to get inputs into the javascript in a usable way, and use javascript to pass information to future steps/jobs

The input can be provided similar to https://github.com/ankidroid/Anki-Android/blob/9036f1611380e2952630527dd26483e9d4772805/.github/workflows/opencollective_notices.yml#L30

We can use @actions/core to set output.

core.setOutput('outputKey', 'outputVal');

To access the value in same job,

${{ steps.[STEP ID].outputs.[KEY] }}

To access the value in different job,

${{ needs.[JOB ID].outputs.[KEY] }}

Defining outputs for jobs

mikehardy commented 2 weeks ago

The toJSON I used but still need to parse the os array because it was still a string inside inputs

The output was just using default return from the script so didn't need anything fancy theree

mikehardy commented 2 weeks ago

Okay, I'm going to merge this now then do the required baby-sitting on branch protection rules

mikehardy commented 2 weeks ago

New branch protection rules are in place. Existing PRs will need a close/reopen cycle to kick off the new CI tests expected by the new rules