Add output to signal if a test result was cached

[joshua-cannon-techlabs]

Is your feature request related to a problem? Please describe.
Coming from Bazelland, it was very nice to see which tests actually ran and which ones didn't due to caching.

Output looks roughly like:

//a/b/c/d:e_test        (cached) PASSED in 29.5s
//a/b:c_test            (cached) PASSED in 1.6s
//a/b/c/d:e2_test                PASSED in 2.8s
//a/b/c:d_test                   PASSED in 16.0s

This raised the confidence in the build tool's ability to cache entire test runs, and in our understanding of the cache invalidation.

Describe the solution you'd like
A similar output for ./pants test (maybe other rules as well, lint/fmt/etc...).

Describe alternatives you've considered
The stats helps, but isn't quite the same.

Additional context
N/A

[joshua-cannon-techlabs]

Additionally, the feature could use different signaling for "memoized" vs. "cached".

[stuhood]

We should definitely do this, yea. In fact, I promised @g-cassie I'd open a ticket about it a while back... oops. Thanks!

[stuhood]

#13590 exposes the information needed to do this. In a followup change, the TestResult type (and any others for which this information might be rendered) should begin carrying along ProcessResultMetadata, and the test goal should request the RunId, and consume the metadata to render cache status and runtime.

[stuhood]

I'm going to have to focus on other things for a bit. Maybe someone else would be interested in picking up the rendering changes, as described above?

[riisi]

@stuhood Had a quick play with this, looks straightforward. But not sure how to get the current process run id though in the test goal - per "the test goal should request the RunId".

[stuhood]

@stuhood Had a quick play with this, looks straightforward. But not sure how to get the current process run id though in the test goal - per "the test goal should request the RunId".

Ah, yea: sorry that that wasn't clear!

#13590 added an intrinsic that exposes RunId as a singleton. To consume a singleton (or any other type which can be computed using what is already in scope), you put it in the argument list of your @rule. So:

@goal_rule
def run_tests(
    console: Console,
    ..,
    run_id: RunId,  # NEW
) -> TestGoal:
    ..

[yoav-orca]

@riisi do you intend to implement this?

[riisi]

Yes, been a bit busy this week but will come back to this tomorrow.

…

On 11 Dec 2021, at 19:00, Yoav Alon ***@***.***> wrote:  @riisi do you intend to implement this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[riisi]

Thoughts on this layout? I'd slightly prefer columns like the Bazel output but this is the least change to get this in, and I don't want to mis-align with the outputs from the other goals - that might require a larger discussion.

The possible cache results are ran_locally, memoized, hit_locally, and hit_remote. Suggest these should be translated for console output as

• <no print output if ran locally>
• (memoized)
• (cached locally)
• (cached remote)

[yoav-orca]

@riisi I would appreciate a sort-by option, where you can get the final output sorted by time

[benjyw]

BTW you absolutely can add columns (in a followup) if you like. There is no requirement that different goal outputs be consistent, and they aren't really today.

[benjyw]

I'd say "cached remotely", so it mirrors "cached locally"?

[Eric-Arellano]

BTW you absolutely can add columns (in a followup) if you like.

A concern I'd have is when your terminal isn't wide enough, that wrapping might look even worse. We intentionally use soft wrapping with the output of test so that the shell handles wrapping for us.

That's probably not a show stopper and I think that columns would be a net win. Although, doing it in a dedicated followup probably makes sense.