[WIP] Run PAPR on newly added test

[mike-nguyen]

Currently PAPR will only run the improved sanity test when there is
a new pull request. This commit will add a very simple script to
run any new test.

[miabbott]

I think this needs to be ./.test_director

See https://github.com/projectatomic/atomic/blob/master/.redhat-ci.yml#L19 as an example

[jlebon]

This can end up being more than a single test though, right? Should we run this in a loop instead? (Though that would require all tests to make sure to cleanly restore the machine to a clean state after they're done). Or maybe it's safer to only ever target a single test and error out if multiple tests are modified to force multiple PRs to be created. Not sure.

[miabbott]

There's been some recent work (#142 for example) to make sure our tests leave the system in the original state. It should be safe to run multiple tests in succession.

[mike-nguyen]

I think we can also end up in a state where a test is deleted. Since we are not checking the status we could be trying to run a test on something that is no longer there. Once I work out some of the current issues and have something working I'll add some more logic into it to handle different scenarios.

[miabbott]

./.test_director: line 4: git: command not found

Looks like we need to add git to the list of packages being installed.

[miabbott]

>>> ./.test_director
++ git diff --name-only master
++ grep 'tests/.*/main.yml'
+ NEW_TEST='tests/fake-test/main.yml
tests/rpm-ostree/main.yml'
+ '[' -z tests/fake-test/main.yml tests/rpm-ostree/main.yml ']'
./.test_director: line 6: [: tests/fake-test/main.yml: binary operator expected

Looks like you better handle the multiple changed tests after all.

Although, if I run .test-director on my own host after pulling this PR, I only get the fake-test change.

[mike-nguyen]

It looks like the diff also includes the last commit that was merged. There must be a difference in the way we're pulling in the PR and PAPR.

[miabbott]

>>> ./.test_director
++ git diff --name-only master
++ grep 'tests/.*/main.yml'
+ NEW_TESTS='tests/fake-test/main.yml
tests/improved-sanity-test/main.yml
tests/rpm-ostree/main.yml'
+ for NEW_TEST in '"${NEW_TESTS[@]}"'
+ '[' -z tests/fake-test/main.yml tests/improved-sanity-test/main.yml tests/rpm-ostree/main.yml ']'
./.test_director: line 7: [: too many arguments

I think you may need to use the PAPR_COMMIT that @jlebon mentioned here

Also, you need a different way to test the NEW_TESTS variable since it could be empty, one file, or multiple files.

[jlebon]

It depends on your needs, but yeah you probably want git diff --name-only origin/master..$PAPR_COMMIT here instead of master. Basically, by default PAPR runs its tests on the merge commit of your PR and master. Because your PR branch is 2 commits behind master, the overall diff is including diffs from those commits as well. If you only want the files modified in your PR, then the above should work!

[mike-nguyen]

Thanks for the ongoing comments as I'm working my way through this 👍

[miabbott]

>>> ./.test_director
+ HEAD=0cb112d11292c10d087b395060b498053ec95c60
+ NEW_TESTS=($(git diff --name-only origin/master..$HEAD | grep 'tests/.*/main.yml' || true))
++ git diff --name-only origin/master..0cb112d11292c10d087b395060b498053ec95c60
++ grep 'tests/.*/main.yml'
+ '[' 2 -eq 0 ']'

This looks like it is doing the right thing now, but I this PR is based on top of 2edd28a so git diff is picking up the rpm-ostree test as part of the PR.

If you rebase the PR onto master (git pull -r upstream master), the next time the checks run, they should only be running the fake-test.

[mike-nguyen]

The last test actually looked like it picked up improved-sanity-test instead of rpm-ostree--which confused me. I added some more info to debug and also simplified the fake-test that runs.

[miabbott]

This looks like it is in a good place. We can add more logic to test changed roles in another PR.

[mike-nguyen]

👍 Thanks for all the comments!

[mike-nguyen]

I think I didn't account for the scenario where the test that is changed is the improved-sanity-test which would run the improved-sanity-test without deploying HEAD-1 first.

[miabbott]

I think I didn't account for the scenario where the test that is changed is the improved-sanity-test which would run the improved-sanity-test without deploying HEAD-1 first.

More ammo to work on #150 sooner rather than later

[miabbott]

(5) Configure [a-h-t] PAPR to validate changed roles/tests