I have a question about filter and minimize minimum set coverage for

go_library

targets. Basically, when I run

filter-minimize

on my repo dir containing my go corpus, the minimized target list is ~33% of the actual list (which could be ok, maybe), and excludes targets that have

go_library

BUILD files and tests. The result is that there are well-defined

go_library

targets that contain tests, which aren’t covered when discovering targets via minimize

minimize computes the covering set of targets, so it's expected that the set is smaller

right, but, it seems it does not meet the covering set criteria

but presumably

./pants test $go_library

is not testing things transitively...

I think so, e.g. one target I found that is not being tested, is listed as a dependency in a target that is tested/output by minimize

@plain-leather-21357: right: if they did, you'd have a lot of redundancy in CI

makes sense, although I don’t have enough context about the graph shape part

@plain-leather-21357: no, would have to add it.

right

the graph shape thing would be to add something like

go_test

(likely automatically...?), and then those would be your minimization roots, and they would have no dependents

ahh, that makes sense

@enough-analyst-54434: guessing you're climbing, but if you have any thoughts later this evening, let us know

note that we should also include

go_binary

in this, as those targets can also have tests

the issue with

./pants test $go_library

testing transitively for go would be that it would cause redundancy when you do the standard pants ci dance: https://www.pantsbuild.org/orgs.html#continuous-integration

That's if we don't implement self.invalidated checking under the v1 engine for single shard testing and if we don't implement test result caching under the v1 engine for sharded testing - which our go support in OSS doesn't support anyhow. So I think adding a transitive flag to

test.go

and implementing a self.invalidated guard is a viable option. Looking at history, the non-transitive nature of the task was an original decision in 2015 by Cody perhaps ripe for revisit.

yea, i suppose so. i guess it's not a huge amount of work to add test caching there, although turning on test caching safely still requires

--chroot

and etc.

How big an effort does this seem like it is? Any idea on when it’d be looked at?

@plain-leather-21357: slightly unclear re:

chrooting

... our internal tickets on the topic indicate that something like 1% of tests fail under chrooting

I muddied things above. Under v1 caching is not a thing here, just invalidation. We could implement this with invalidation only.

caching is relevant in a distributed environment though

But v1 and go test does not support sharding - right - so it doesn;'t matter until v2

and we do, because we do https://www.pantsbuild.org/orgs.html#continuous-integration with the for-loop replaced with parallel/distributed test running

k

ok, i think we have the information to create a ticket for this one. thanks everyone!

Injecting go_test targets for the purpose of getting minimize to work definitely seems hacky. Adjusting the ci script to handle go seperately, turning minimial cover into all interior targets too, then adding that to the list - seems preferrable from a pants end but probably not your end.