most assets have unit tests but CI infra doesn't run them

[jaketf]

@morgante @AdrienWalkowiak @ryanmcdowell would love your thoughts on how we might tackle this.

Issue

Contributors write tests that are never run and sometimes we merge changes that break tests. we should have a part of the CI process for running tests in these assets.

Background

Mostly the repo seems to be (mostly) standardized on mvn test python unittest go test

Future state

Test Discovery

Ideally we'd have a test discovery / running script in the CI tool similar to check_format.sh However this is difficult to implement because each example / tool has different dependencies and sometimes use different build tools (npm, sbt, gradle, etc.)

New Asset Bootstrapper

If we can align on approach we can write a "new tool / new example generator script that seeds a new directory w/ a template Make / Docker file, README, etc."

Additional CI Check on PRs

For "every dir under tools / examples has Make / Docker / OWNERS / README

More strict contribution guidelines requirements

I'd be interested in modifying the contribution guidelines so every new asset must have a Dockerfile / Makefile to run tests. This way discovery tool could just run make test in each directory. This would also make reviewers job easier to pull down and run tests (which I admittedly don't usually do).

Additionally, it feels like as we try to improve the quality / maintainability of this repo we should have an OWNERS file under each tool / example. It can be difficult the track down original authors to triage issues and it'd be nice to reach out to all owners for repo wide initiatives and say...

Messaging

What's to best way to get this message out there (and do we agree with it): "Hey we all need to migrate out of exclusions list, add docker / make / owner files by end of 2020 and upgrade to python3 or your asset will be considered unmaintained and dropped from the repo"

[morgante]

I think adding a Makefile to each directory is a good idea, we could easily update CI to run make test for each of those.

+1 to adding a new tool bootstrapper. I'd recommend using cookiecutter as a simple solution to add this.

In terms of rolling this out, I suggest we start by writing up a brief standards/contributing guide (or add it to go/pso-github-process). We can send out a call to action for PSO once that's written.

[ryanmcdowell]

Agreed with the above proposal.

Is there a reason why we wouldn't require each contribution to have it's own cloudbuild file? This is how I've seen this managed elsewhere where the parent build file can kickoff child builds (https://github.com/GoogleCloudPlatform/cloud-builders-community).

Totally agree with having an OWNERS file. Are you imagining GitHub ids or LDAPs?

[morgante]

Cloud Build files could also be good, as we could configure triggers to only run the tests when that example is touched.

Totally agree with having an OWNERS file. Are you imagining GitHub ids or LDAPs?

I'd suggest GitHub CODEOWNERS.

[tswast]

Note: CODEOWNERS also supports GitHub teams. That may be a more scalable solution.

[ryanmcdowell]

SGTM

[tswast]

Re: running in each sub-dir, it doesn't appear to be natively supported. It required some bash magic here: https://github.com/GoogleCloudPlatform/cloud-builders-community/blob/master/cloudbuild.yaml but should be easy enough to adapt 🤞

[jaketf]

Speaking of bash magic.... A super easy win in this space would be adding shellcheck and terraform fmt to the CI tests. I recently had to do something similar in an adaptation of what we do in this repo here

[jaketf]

notably the helper scripts in this repo don't even pass shellcheck tho!