> On Oct 13, 2020, at 11:46 AM, Jarek Potiuk <jarek.pot...@polidea.com> wrote:
>
> I am rather interested in how those kinds of cases might be handled better
> by Yetus - i.e. how much smarter it can be when selecting which parts of
> the tests should be run - and how you would define such relation. What
> pre-commit is doing is rather straightforward (run tests on files that
> changed), what I did in tests takes into account the "structure" of the
> project and acts accordingly. And those are rather simple to implement. As
> you'd see in my PR it's merely <100 lines in bash to find which files have
> changed and based on some predefined rules select which tests to run. I'd
> be really interested though if Yates can provide some better ways of
> handling it?
I think you are misunderstanding where Yetus sits in the stack. And I
also misunderstood where you were running pre-commit; it's clear you aren't
running it _also_ as part of the CI, just as part of the developer experience.
(which also means there is an assumption that every PR _has_ run those tools
...)
Yetus' precommit probably better thought about as a pre-merge
utility. The functionality pre-dates git and commit hooks sooooo...
Anyway, let's look at Airflow. For example, in Airflow's CI is
static-checks-pylint
(https://github.com/apache/airflow/blob/master/.github/workflows/ci.yml). This
runs on _every_ PR.
Let's look at https://github.com/apache/airflow/pull/11518. This is a markdown
update. There is no python code. Yet:
https://github.com/apache/airflow/runs/1250824427?check_suite_focus=true
16 minutes blown on an absolutely useless check. And that's just pylint. Look
at the entire CI Build workflow:
https://github.com/apache/airflow/actions/runs/305454304
~45 minutes for likely zero value. Spell check might be the only thing that
happened that was actual useful. That's 45 minutes that something else could
have been executing. I haven't looked at the other workflows that also ran but
probably more wasted time.
Under Yetus, test-patch would have detected this PR was markdown, ran
markdownlint, blanks, and probably a few other things, and reported the
results. It probably would have taken _maybe_ 2 minutes, most of that spent
dealing with the docker image. Hooray! 43 minutes back in the executor pool
for either more Airflow work or for another project!
Is this an extreme example? Sure. But if you stretch these types of
cuts over a large amount of PRs, it makes a huge, huge difference.
Airflow is 'advanced' enough in its CI that using test-patch to cut
back on _all_ of these workflows is certainly possible using custom plug-ins.
But it might be easier to use smart-apply-patch's --changedfilesreport option
to generate a list of files in the PR and then short-circuit workflows based
upon that list of file changes. (which reminds me, we need to update the
smart-apply-patch docs to cover this haha)
===
In the specific case of testing, test-patch will slice and dice tests
based upon the build tool. So if you are using, say, maven, it will only run
the unit tests for that module. There is nothing an end user needs to do. No
classifications or anything like that. They get that functionality for free.
Since Airflow doesn't have a build tool that Yetus supports unfortunately. So
it wouldn't work out of the box, but it could be shoe-horned in by supplying a
custom build-tool. Probably not worth the effort in this specific use case,
frankly.
