Compute the SCCs of tests as well #8899
Conversation
There was a problem hiding this comment.
Good thing we made the SCC computation 2x faster a while back! :pats-self-on-back:
I think this change can land first, but will need to be modified after @aisamanra 's test import changes land?
| switch (edgeType) { | ||
| case core::packages::ImportType::Normal: { | ||
| poppedPkgInfo.sccID_ = sccId; | ||
| break; | ||
| } | ||
| case core::packages::ImportType::Test: { | ||
| poppedPkgInfo.testSccID_ = sccId; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we make it so that we can use a pointer-to-member here, since the SCC we're modifying is constant over the entire tarjan run? I don't have super-strong opinions on the right way to pass that in at the top-level, but I think the right direction is somehow to link the edgeType to the pointer-to-member that we want? Maybe a method on PackageInfo that does that?
That might be Too Much C++, but my sense is that saving a branch here is valuable.
There was a problem hiding this comment.
Yeah, I think that's a good idea. I'll have a look at how we might do that at the tarjan call site.
Yes... we'll need to run it a third time |
Currently, we ignore test dependencies when computing SCCs for packages. This is fine for our current needs, as we don't enforce strict dependencies or layering for test code, but this will change soon when we start refactoring the pipeline to operate on SCCs of packages.
This PR adds a second scc id to each package,
testSccID. The new ID is the result of a separate run of Tarjan's algorithm over just test dependencies. While it's not used for anything currently, it will soon be used to emit a possible traversal order for packages when typechecking by SCC.This change roughly doubles the time to compute SCCs, as it's now running the algorithm twice over the package graph.
Motivation
Prework for typechecking by package SCC.
Test plan
Existing tests.