Require explicit ExecKind in SpawnMetrics.Builder and remove SpawnMetrics.EMPTY
Previously we used `REMOTE` as the default but now we'll require setting
`ExecKind` explicitly. To make it less error prone, I've introduced a few
factory methods that set it. Also, as previously discussed, I've introduced
a new `ExecKind.OTHER` (IMHO sounds a bit better than the initial proposal
of `INTERNAL`, but I don't feel strongly, so let me know if you prefer
the latter).
We also discussed not setting `ExecKind` in `SpawnMetrics.EMPTY` (and using
something like `null`), but I found it a bit tricky to really define what
that would actually mean, now that we have `ExecKind` (e.g., should we allow
aggregating such "empty" values? if yes, should it be commutative? etc.)
But I've noticed that after the introduction of `AggregatedSpawnMetrics`,
we only use `EMPTY` in one place and it's possible to simply remove it, which
avoids the problem.
RELNOTES: None.
PiperOrigin-RevId: 314687611
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index ad1f45f..aac8abd 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -975,7 +975,7 @@
@Test
public void accountingAddsDurationsForStages() {
SpawnMetrics.Builder builder =
- new SpawnMetrics.Builder()
+ SpawnMetrics.Builder.forRemoteExec()
.setQueueTime(Duration.ofSeconds(1))
.setSetupTime(Duration.ofSeconds(2))
.setExecutionWallTime(Duration.ofSeconds(2))
@@ -1026,10 +1026,7 @@
}
private RemoteSpawnRunner newSpawnRunner() {
- return newSpawnRunner(
- executor,
- /* reporter= */ null,
- /* topLevelOutputs= */ ImmutableSet.of());
+ return newSpawnRunner(executor, /* reporter= */ null, /* topLevelOutputs= */ ImmutableSet.of());
}
private RemoteSpawnRunner newSpawnRunner(Reporter reporter) {
