Skip to content

Commit 65b9273

Browse files
committed
Don't error on some aborted events
Some BEP events, such as - TargetConfigured - TargetCompleted - TestSummary - TestResult may be sent with an aborted payload instead of a normal payload, indicating that the relevant event will never be sent. bb-portal handled this for TargetCompleted events, but not for the others. This caused errors during BEP processing when such aborted events were encountered that were propagated up to the client. This change updates the BEP processing code to ignore aborted events for TargetConfigured, TestSummary, and TestResult events, since they have no relevant data. For TargetCompleted events, the existing handling is kept, to allow for relevant targets to be shown for bazel queries.
1 parent ad84968 commit 65b9273

File tree

20 files changed

+10898
-28
lines changed

20 files changed

+10898
-28
lines changed

ent/gen/ent/mutation.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ent/gen/ent/testsummary.go

Lines changed: 7 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ent/gen/ent/testsummary_create.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ent/schema/testsummary.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (TestSummary) Fields() []ent.Field {
4949
field.Time("last_stop_time").Optional(),
5050

5151
// The total runtime of the test.
52-
field.Int64("total_run_duration_in_ms").Optional().
52+
field.Int64("total_run_duration_in_ms").Optional().Nillable().
5353
Annotations(entgql.OrderField("TOTAL_RUN_DURATION_IN_MS")),
5454
}
5555
}

frontend/src/components/TestTab/index.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,13 @@ export const TestTab: React.FC<Props> = ({ invocationId }) => {
101101

102102
const parsedData: TestTabRowType[] = useMemo(() => {
103103
return parseGraphqlEdgeList(data?.findTestSummaries).map((ts) => {
104-
console.table(ts);
105104
var cachedLocally: boolean | null = null;
106105
var cachedRemotely: boolean | null = null;
107-
if (ts.testResults !== null && ts.testResults !== undefined) {
106+
if (
107+
ts.testResults !== null &&
108+
ts.testResults !== undefined &&
109+
ts.testResults.length > 0
110+
) {
108111
if (ts.testResults.every((tr) => tr.cachedLocally === true)) {
109112
cachedLocally = true;
110113
} else if (ts.testResults.some((tr) => tr.cachedLocally === false)) {

internal/database/buildeventrecorder/save_target_configured.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,20 @@ func (r *BuildEventRecorder) saveTargetConfiguredBatch(ctx context.Context, batc
5959
return status.Error(codes.InvalidArgument, "Attempted to configure targets for an invocation but the invocation was already completed.")
6060
}
6161

62-
targetIds, err := getOrCreateTargets(ctx, r.InstanceNameDbID, tx, batch)
62+
// Don't handle aborted events.
63+
filteredBatch := make([]BuildEventWithInfo, 0, len(batch))
64+
for _, x := range batch {
65+
if x.Event.GetConfigured() != nil {
66+
filteredBatch = append(filteredBatch, x)
67+
}
68+
}
69+
70+
targetIds, err := getOrCreateTargets(ctx, r.InstanceNameDbID, tx, filteredBatch)
6371
if err != nil {
6472
return util.StatusWrap(err, "Failed to get or create targets")
6573
}
6674

67-
if err := createTargetKindMappingsBulk(ctx, r.IsRealTime, r.InvocationDbID, tx, batch, targetIds); err != nil {
75+
if err := createTargetKindMappingsBulk(ctx, r.IsRealTime, r.InvocationDbID, tx, filteredBatch, targetIds); err != nil {
6876
return util.StatusWrap(err, "Failed to bulk insert event metadata")
6977
}
7078

internal/database/buildeventrecorder/save_test_result.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,15 @@ func (r *BuildEventRecorder) saveTestResultBatch(ctx context.Context, batch []Bu
5959
return status.Error(codes.InvalidArgument, "Attempted to configure targets for an invocation but the invocation was already completed.")
6060
}
6161

62-
if err := createTestResultsBulk(ctx, r.InvocationDbID, r.InstanceNameDbID, tx, batch); err != nil {
62+
// Don't handle aborted events.
63+
filteredBatch := make([]BuildEventWithInfo, 0, len(batch))
64+
for _, x := range batch {
65+
if x.Event.GetTestResult() != nil {
66+
filteredBatch = append(filteredBatch, x)
67+
}
68+
}
69+
70+
if err := createTestResultsBulk(ctx, r.InvocationDbID, r.InstanceNameDbID, tx, filteredBatch); err != nil {
6371
return util.StatusWrap(err, "Failed to bulk insert test results")
6472
}
6573

internal/database/buildeventrecorder/save_test_summary.go

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,31 @@ func (r *BuildEventRecorder) saveTestSummaryBatch(ctx context.Context, batch []B
5757
return status.Error(codes.InvalidArgument, "Attempted to configure targets for an invocation but the invocation was already completed.")
5858
}
5959

60+
// Don't handle aborted events.
61+
filteredBatch := make([]BuildEventWithInfo, 0, len(batch))
62+
for _, x := range batch {
63+
if x.Event.GetTestSummary() != nil {
64+
filteredBatch = append(filteredBatch, x)
65+
}
66+
}
67+
6068
params := sqlc.UpdateTestSummariesBulkParams{
6169
BazelInvocationID: int64(r.InvocationDbID),
6270
InstanceNameID: int64(r.InstanceNameDbID),
63-
Labels: make([]string, len(batch)),
64-
ConfigIds: make([]string, len(batch)),
65-
OverallStatuses: make([]string, len(batch)),
66-
TotalRunCounts: make([]int32, len(batch)),
67-
RunCounts: make([]int32, len(batch)),
68-
AttemptCounts: make([]int32, len(batch)),
69-
ShardCounts: make([]int32, len(batch)),
70-
TotalNumCacheds: make([]int32, len(batch)),
71-
StartTimes: make([]time.Time, len(batch)),
72-
StopTimes: make([]time.Time, len(batch)),
73-
Durations: make([]int64, len(batch)),
71+
Labels: make([]string, len(filteredBatch)),
72+
ConfigIds: make([]string, len(filteredBatch)),
73+
OverallStatuses: make([]string, len(filteredBatch)),
74+
TotalRunCounts: make([]int32, len(filteredBatch)),
75+
RunCounts: make([]int32, len(filteredBatch)),
76+
AttemptCounts: make([]int32, len(filteredBatch)),
77+
ShardCounts: make([]int32, len(filteredBatch)),
78+
TotalNumCacheds: make([]int32, len(filteredBatch)),
79+
StartTimes: make([]time.Time, len(filteredBatch)),
80+
StopTimes: make([]time.Time, len(filteredBatch)),
81+
Durations: make([]int64, len(filteredBatch)),
7482
}
7583

76-
for i, x := range batch {
84+
for i, x := range filteredBatch {
7785
be := x.Event
7886
testSummaryID := be.GetId().GetTestSummary()
7987
testSummary := be.GetTestSummary()
@@ -114,7 +122,7 @@ func (r *BuildEventRecorder) saveTestSummaryBatch(ctx context.Context, batch []B
114122
if err != nil {
115123
return util.StatusWrap(err, "Failed to bulk update test summaries")
116124
}
117-
if affectedRows != int64(len(batch)) {
125+
if affectedRows != int64(len(filteredBatch)) {
118126
return status.Errorf(codes.Internal, "Expected to update %d test summaries, but only updated %d", len(batch), affectedRows)
119127
}
120128

internal/graphql/server_gen.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integrationtest/integration_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ var (
6060
filename: "nextjs_analysis_fail.bep.ndjson",
6161
invocationID: "df7178e2-a815-4654-a409-d18e845d1e35",
6262
}
63+
abortedAnalysis = bepFile{
64+
filename: "bb_portal_no_analyze.bep.ndjson",
65+
invocationID: "bb2b45d9-d695-4194-8fdc-01ba50477a92",
66+
}
67+
abortedBuild = bepFile{
68+
filename: "bb_portal_no_build.bep.ndjson",
69+
invocationID: "76651929-40d8-4f79-a2de-6a4a5092b76c",
70+
}
71+
abortedTests = bepFile{
72+
filename: "bb_portal_aborted_tests.ndjson",
73+
invocationID: "64719226-555e-494d-9918-0fd25d468b1e",
74+
}
6375
authenticatedUserUUID = "8bdb3187-e36c-487e-95b8-f8ca28a82068"
6476

6577
// An authenticated user UUID not present in any BEP file.
@@ -98,6 +110,9 @@ var (
98110
{bepFile: successfulBazelTest},
99111
{bepFile: failedBazelTest},
100112
{bepFile: failedBazelAnalysis},
113+
{bepFile: abortedAnalysis},
114+
{bepFile: abortedBuild},
115+
{bepFile: abortedTests},
101116
},
102117
graphqlTestCases: graphqlTestTable{
103118
"LoadFullBazelInvocationDetails": {
@@ -132,6 +147,21 @@ var (
132147
},
133148
wantErr: errInvocationNotFound,
134149
},
150+
"get aborted analysis invocation": {
151+
variables: testkit.Variables{
152+
"invocationID": abortedAnalysis.invocationID,
153+
},
154+
},
155+
"get aborted build invocation": {
156+
variables: testkit.Variables{
157+
"invocationID": abortedBuild.invocationID,
158+
},
159+
},
160+
"get aborted tests invocation": {
161+
variables: testkit.Variables{
162+
"invocationID": abortedTests.invocationID,
163+
},
164+
},
135165
},
136166
"FindBuildByUUID": {
137167
"found": {
@@ -168,6 +198,16 @@ var (
168198
"invocationID": failedBazelAnalysis.invocationID,
169199
},
170200
},
201+
"get targets for aborted analysis": {
202+
variables: testkit.Variables{
203+
"invocationID": abortedAnalysis.invocationID,
204+
},
205+
},
206+
"get targets for aborted build": {
207+
variables: testkit.Variables{
208+
"invocationID": abortedBuild.invocationID,
209+
},
210+
},
171211
},
172212
"GetTargetDetails": {
173213
"get details for a target": {
@@ -208,6 +248,13 @@ var (
208248
},
209249
},
210250
},
251+
"GetTestsForInvocation": {
252+
"get tests for aborted tests": {
253+
variables: testkit.Variables{
254+
"invocationID": abortedTests.invocationID,
255+
},
256+
},
257+
},
211258
},
212259
},
213260
{

0 commit comments

Comments
 (0)