Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set a ceiling on the number of extra files to attach. #16653

Merged
merged 1 commit into from
Mar 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion experiment/resultstore/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func convertSuiteMeta(suiteMeta gcs.SuitesMeta) resultstore.Suite {
}

// Convert converts build metadata stored in gcp into the corresponding ResultStore Invocation, Target and Test.
func convert(project, details string, url gcs.Path, result downloadResult) (resultstore.Invocation, resultstore.Target, resultstore.Test) {
func convert(project, details string, url gcs.Path, result downloadResult, maxFiles int) (resultstore.Invocation, resultstore.Target, resultstore.Test) {
started := result.started
finished := result.finished
artifacts := result.artifactURLs
Expand Down Expand Up @@ -175,13 +175,18 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
artifacts[i] = "gs://" + bucket + "/" + a
}

var total int
for _, a := range artifacts { // add started.json, etc to the invocation artifact list.
if total >= maxFiles {
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log somewhere when we hit this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
if strings.HasPrefix(a, artifactsPath) {
continue // things under artifacts/ are owned by the test
}
if a == buildLog {
continue // Handle this in InvocationLog
}
total++
inv.Files = append(inv.Files, resultstore.File{
ID: uniqPath(a),
ContentType: "text/plain",
Expand Down Expand Up @@ -237,6 +242,9 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
}

for _, a := range artifacts {
if total >= maxFiles {
continue
}
if !strings.HasPrefix(a, artifactsPath) {
continue // Non-artifacts (started.json, etc) are owned by the invocation
}
Expand All @@ -254,13 +262,23 @@ func convert(project, details string, url gcs.Path, result downloadResult) (resu
if found {
continue
}
total++
test.Suite.Files = append(test.Suite.Files, resultstore.File{
ID: uniqPath(a),
ContentType: "text/plain",
URL: a,
})
}

if total >= maxFiles {
// TODO(fejta): expose this to edge case to user in a better way
inv.Files = append(inv.Files, resultstore.File{
ID: fmt.Sprintf("exceeded %d files", maxFiles),
ContentType: "text/plain",
URL: basePath,
})
}

test.Suite.Start = inv.Start
test.Action.Start = inv.Start
test.Suite.Duration = inv.Duration
Expand Down
193 changes: 192 additions & 1 deletion experiment/resultstore/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
details string
url string
result downloadResult
maxFiles int
expectedInvocation resultstore.Invocation
expectedTarget resultstore.Target
expectedTest resultstore.Test
Expand Down Expand Up @@ -396,6 +397,193 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
},
},
},
{
name: "Reject excessive artifacts",
maxFiles: 3,
project: "projectX",
details: "detailY",
url: "gs://bucket/logs/jobA/1234567890123456789",
result: downloadResult{
started: gcs.Started{
Started: metadata.Started{
Timestamp: 1234567890,
Repos: map[string]string{
"org/repoA": "branchB",
},
DeprecatedRepoVersion: "aadb2b88d190a38b59f512b4d8c508a88cf839e1",
},
Pending: false,
},
finished: gcs.Finished{
Finished: metadata.Finished{
Result: "SUCCESS",
DeprecatedRevision: "master",
},
Running: false,
},
artifactURLs: []string{
"logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
"logs/jobA/1234567890123456789/artifacts/1",
"logs/jobA/1234567890123456789/artifacts/2",
"logs/jobA/1234567890123456789/artifacts/3",
"logs/jobA/1234567890123456789/artifacts/4",
"logs/jobA/1234567890123456789/artifacts/5",
"logs/jobA/1234567890123456789/build-log.txt",
},
suiteMetas: []gcs.SuitesMeta{
{
Suites: junit.Suites{
XMLName: xml.Name{},
Suites: []junit.Suite{
{
XMLName: xml.Name{Space: "testsuite", Local: ""},
Time: 10.5,
Failures: 0,
Tests: 2,
Results: []junit.Result{
{
Name: "Result1",
Time: 3.2,
ClassName: "test1",
Properties: &junit.Properties{
PropertyList: []junit.Property{
{Name: "p1", Value: "v1"},
},
},
},
{
Name: "Result2",
Time: 7.3,
ClassName: "test2",
Properties: &junit.Properties{
PropertyList: []junit.Property{
{Name: "p2", Value: "v2"},
},
},
},
},
},
},
},
Metadata: map[string]string{
"Context": "runner",
},
Path: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
},
},
expectedInvocation: resultstore.Invocation{
Project: "projectX",
Details: "detailY",
Files: []resultstore.File{
{
ID: resultstore.InvocationLog,
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/build-log.txt",
},
{
ID: "exceeded 3 files",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/",
},
},
Properties: []resultstore.Property{
{Key: "Job", Value: "jobA"},
{Key: "Pull", Value: ""},
{Key: "Org", Value: "org"},
{Key: "Branch", Value: "branchB"},
{Key: "Repo", Value: "repoA"},
{Key: "Repo", Value: "org/repoA"},
{Key: "Repo", Value: "org/repoA:branchB"},
},
Start: time.Unix(1234567890, 0),
Status: resultstore.Running,
Description: "In progress...",
},
expectedTarget: resultstore.Target{
Status: resultstore.Running,
Description: "In progress...",
Start: time.Unix(1234567890, 0),
Properties: []resultstore.Property{
{Key: "Result1:p1", Value: "v1"},
{Key: "Result2:p2", Value: "v2"},
},
},
expectedTest: resultstore.Test{
Suite: resultstore.Suite{
Name: "test",
Start: time.Unix(1234567890, 0),
Files: []resultstore.File{
{
ID: resultstore.TargetLog,
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/build-log.txt",
},
{
ID: "artifacts/junit_runner.xml",
ContentType: "text/xml",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
{
ID: "artifacts/1",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/1",
},
{
ID: "artifacts/2",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/2",
},
{
ID: "artifacts/3",
ContentType: "text/plain",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/3",
},
},
Suites: []resultstore.Suite{
{
Name: "junit_runner.xml",
Duration: dur(10.5),
Files: []resultstore.File{
{
ID: "junit_runner.xml",
ContentType: "text/xml",
URL: "gs://bucket/logs/jobA/1234567890123456789/artifacts/junit_runner.xml",
},
},
Suites: []resultstore.Suite{
{
Cases: []resultstore.Case{
{
Name: "Result1",
Class: "test1",
Result: resultstore.Completed,
Duration: dur(3.2),
},
{
Name: "Result2",
Class: "test2",
Result: resultstore.Completed,
Duration: dur(7.3),
},
},
Duration: dur(10.5),
Properties: []resultstore.Property{
{Key: "Result1:p1", Value: "v1"},
{Key: "Result2:p2", Value: "v2"},
},
},
},
},
},
},
Action: resultstore.Action{
Start: time.Unix(1234567890, 0),
Status: resultstore.Running,
Description: "In progress...",
},
},
},
{
name: "Convert full project metadata",
project: "projectX",
Expand Down Expand Up @@ -566,7 +754,10 @@ func TestConvertProjectMetadataToResultStoreArtifacts(t *testing.T) {
if err != nil {
t.Errorf("incorrect url: %v", err)
}
invocation, target, test := convert(tc.project, tc.details, *urlPath, tc.result)
if tc.maxFiles == 0 {
tc.maxFiles = 40000
}
invocation, target, test := convert(tc.project, tc.details, *urlPath, tc.result, tc.maxFiles)
if diff := cmp.Diff(invocation, tc.expectedInvocation, cmpOption); diff != "" {
t.Errorf("%s:%s mismatch (-got +want):\n%s", tc.name, "invocation", diff)
}
Expand Down
Loading