diff --git a/lib/gh/download_file_from_release.go b/lib/gh/download_file_from_release.go index 69f39b22c..8fd6c4ec1 100644 --- a/lib/gh/download_file_from_release.go +++ b/lib/gh/download_file_from_release.go @@ -18,9 +18,12 @@ import ( "context" "errors" "io" + "log/slog" "net/http" + "strings" "github.com/google/go-github/v73/github" + "golang.org/x/mod/semver" ) var ( @@ -31,11 +34,22 @@ var ( ErrFatalError = errors.New("fatal error using github") ) +// ReleaseFile represents a file in a given Github release. +type ReleaseFile struct { + Contents io.ReadCloser + Info ReleaseInfo +} + +type ReleaseInfo struct { + // If the tag is valid, the will be non null. + Tag *string +} + func (c *Client) DownloadFileFromRelease( ctx context.Context, owner, repo string, httpClient *http.Client, - filePattern string) (io.ReadCloser, error) { + filePattern string) (*ReleaseFile, error) { release, _, err := c.repoClient.GetLatestRelease(ctx, owner, repo) if err != nil { // nolint: exhaustruct // WONTFIX. This is an external package. Cannot control it. @@ -84,5 +98,24 @@ func (c *Client) DownloadFileFromRelease( return nil, errors.Join(ErrUnableToDownloadAsset, err) } - return resp.Body, nil + // Returns a tag or empty string if not found. + tagName := release.GetTagName() + // In the event the tag is missing the prefix "v", add it. + // According https://pkg.go.dev/golang.org/x/mod/semver, the version must start with v + if len(tagName) > 0 && !strings.HasPrefix(tagName, "v") { + tagName = "v" + tagName + } + var tagNamePtr *string + if semver.IsValid(tagName) { + tagNamePtr = &tagName + } else { + slog.WarnContext(ctx, "invalid tag. it will not be used", "tag", tagName) + } + + return &ReleaseFile{ + Contents: resp.Body, + Info: ReleaseInfo{ + Tag: tagNamePtr, + }, + }, nil } diff --git a/lib/gh/download_file_from_release_test.go b/lib/gh/download_file_from_release_test.go index c2f34acd2..fe3d19d71 100644 --- a/lib/gh/download_file_from_release_test.go +++ b/lib/gh/download_file_from_release_test.go @@ -21,6 +21,7 @@ import ( "net/http" "testing" + "github.com/google/go-cmp/cmp" "github.com/google/go-github/v73/github" ) @@ -34,6 +35,12 @@ func checkIfFileIsReadable(t *testing.T, file io.Reader) { } } +func checkIfTagIsPopulated(t *testing.T, file *ReleaseFile) { + if file.Info.Tag == nil || *file.Info.Tag == "" { + t.Error("tag is empty") + } +} + func TestDownloadFileFromReleaseWebFeatures(t *testing.T) { t.Skip("Used for debugging purposes.") client := NewClient("") @@ -44,14 +51,14 @@ func TestDownloadFileFromReleaseWebFeatures(t *testing.T) { t.Errorf("unexpected error: %s", err.Error()) } else { // Close to be safe at the end. - defer file.Close() - checkIfFileIsReadable(t, file) + defer file.Contents.Close() + checkIfFileIsReadable(t, file.Contents) + checkIfTagIsPopulated(t, file) } } func TestDownloadFileFromReleaseBrowserCompatData(t *testing.T) { t.Skip("Used for debugging purposes.") - t.Skip("Cannot remove until https://github.com/mdn/browser-compat-data/issues/22675 is fixed") client := NewClient("") ctx := context.Background() httpClient := http.DefaultClient @@ -60,8 +67,9 @@ func TestDownloadFileFromReleaseBrowserCompatData(t *testing.T) { t.Errorf("unexpected error: %s", err.Error()) } else { // Close to be safe at the end. - defer file.Close() - checkIfFileIsReadable(t, file) + defer file.Contents.Close() + checkIfFileIsReadable(t, file.Contents) + checkIfTagIsPopulated(t, file) } } @@ -113,6 +121,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { cfg mockGetLatestReleaseConfig roundTripCfg *mockRoundTripperConfig expectedError error + expectedFile *ReleaseFile }{ { name: "successful download", @@ -127,6 +136,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { BrowserDownloadURL: valuePtr("http://example.com/file.txt"), }, }, + TagName: valuePtr("v1.0.0"), }, err: nil, }, @@ -135,10 +145,51 @@ func TestMockDownloadFileFromRelease(t *testing.T) { //nolint: exhaustruct resp: &http.Response{ StatusCode: http.StatusOK, + Body: io.NopCloser(nil), }, err: nil, }, expectedError: nil, + expectedFile: &ReleaseFile{ + Contents: io.NopCloser(nil), + Info: ReleaseInfo{ + Tag: valuePtr("v1.0.0"), + }, + }, + }, + { + name: "successful download without leading v in tag", + cfg: mockGetLatestReleaseConfig{ + expectedOwner: "owner", + expectedRepo: "repo", + //nolint: exhaustruct + release: &github.RepositoryRelease{ + Assets: []*github.ReleaseAsset{ + { + Name: valuePtr("file.txt"), + BrowserDownloadURL: valuePtr("http://example.com/file.txt"), + }, + }, + TagName: valuePtr("2.0.0"), + }, + err: nil, + }, + roundTripCfg: &mockRoundTripperConfig{ + expectedURL: "http://example.com/file.txt", + //nolint: exhaustruct + resp: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(nil), + }, + err: nil, + }, + expectedError: nil, + expectedFile: &ReleaseFile{ + Contents: io.NopCloser(nil), + Info: ReleaseInfo{ + Tag: valuePtr("v2.0.0"), + }, + }, }, { name: "rate limit with github api", @@ -151,6 +202,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { }, roundTripCfg: nil, expectedError: ErrRateLimit, + expectedFile: nil, }, { name: "unknown error with github api", @@ -162,6 +214,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { }, roundTripCfg: nil, expectedError: ErrFatalError, + expectedFile: nil, }, { name: "missing asset", @@ -174,6 +227,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { }, roundTripCfg: nil, expectedError: ErrAssetNotFound, + expectedFile: nil, }, { name: "request.DO() fails", @@ -198,6 +252,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { err: errors.New("something went wrong"), }, expectedError: ErrUnableToDownloadAsset, + expectedFile: nil, }, { name: "failed to download", @@ -224,6 +279,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { err: nil, }, expectedError: ErrUnableToDownloadAsset, + expectedFile: nil, }, } for _, tc := range testCases { @@ -241,7 +297,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) { httpClient := http.DefaultClient httpClient.Transport = &rt - _, err := client.DownloadFileFromRelease( + file, err := client.DownloadFileFromRelease( context.Background(), "owner", "repo", @@ -251,6 +307,9 @@ func TestMockDownloadFileFromRelease(t *testing.T) { if !errors.Is(err, tc.expectedError) { t.Errorf("unexpected error expected: %v received: %v", tc.expectedError, err) } + if diff := cmp.Diff(tc.expectedFile, file); diff != "" { + t.Errorf("unexpected file (-want +got):\n%s", diff) + } }) } } diff --git a/lib/go.mod b/lib/go.mod index 4acc21a3e..44cf3274a 100644 --- a/lib/go.mod +++ b/lib/go.mod @@ -68,6 +68,7 @@ require ( go.opentelemetry.io/otel/exporters/stdout/stdoutlog v0.13.0 // indirect go.opentelemetry.io/otel/log v0.13.0 // indirect go.opentelemetry.io/otel/sdk/log v0.13.0 // indirect + golang.org/x/mod v0.27.0 // indirect google.golang.org/appengine/v2 v2.0.6 // indirect ) diff --git a/lib/go.sum b/lib/go.sum index dfec48d99..a34195f85 100644 --- a/lib/go.sum +++ b/lib/go.sum @@ -1253,6 +1253,8 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91 golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ= +golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= diff --git a/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker.go b/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker.go index f16d6f884..96c3052e9 100644 --- a/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker.go +++ b/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker.go @@ -20,6 +20,7 @@ import ( "net/http" "github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/bcdconsumertypes" + "github.com/GoogleChrome/webstatus.dev/lib/gh" "github.com/GoogleChrome/webstatus.dev/workflows/steps/services/bcd_consumer/pkg/data" ) @@ -45,7 +46,7 @@ type DataGetter interface { ctx context.Context, owner, repo string, httpClient *http.Client, - filePattern string) (io.ReadCloser, error) + filePattern string) (*gh.ReleaseFile, error) } // DataParser describes the behavior to read raw bytes into the expected BCDData struct. @@ -106,7 +107,7 @@ func (p BCDJobProcessor) Process( } // Step 2. Parse the file. - data, err := p.dataParser.Parse(file) + data, err := p.dataParser.Parse(file.Contents) if err != nil { return err } diff --git a/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker_test.go b/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker_test.go index 1dfbe61bf..dd6eeb384 100644 --- a/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker_test.go +++ b/workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker_test.go @@ -27,6 +27,7 @@ import ( "github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/bcdconsumertypes" "github.com/GoogleChrome/webstatus.dev/lib/gen/jsonschema/mdn__browser_compat_data" + "github.com/GoogleChrome/webstatus.dev/lib/gh" "github.com/GoogleChrome/webstatus.dev/workflows/steps/services/bcd_consumer/pkg/data" ) @@ -56,7 +57,7 @@ type mockDownloadFileFromReleaseConfig struct { repoOwner string repoName string filePattern string - fakeFile io.ReadCloser + fakeFile *gh.ReleaseFile err error } @@ -69,7 +70,7 @@ func (m *MockDataGetter) DownloadFileFromRelease( _ context.Context, owner, repo string, _ *http.Client, - filePattern string) (io.ReadCloser, error) { + filePattern string) (*gh.ReleaseFile, error) { if m.mockDownloadFileFromReleaseCfg.repoOwner != owner || m.mockDownloadFileFromReleaseCfg.repoName != repo || m.mockDownloadFileFromReleaseCfg.filePattern != filePattern { @@ -97,7 +98,8 @@ func (m *MockDataParser) Parse(in io.ReadCloser) (*data.BCDData, error) { m.t.Errorf("unable to read file") } if m.mockParseCfg.expectedFileContents != string(fileContents) { - m.t.Error("unexpected file contents") + m.t.Errorf("unexpected file contents. want: %s, got: %s", + m.mockParseCfg.expectedFileContents, string(fileContents)) } return m.mockParseCfg.ret, m.mockParseCfg.err @@ -205,6 +207,16 @@ func getSampleReleases() []bcdconsumertypes.BrowserRelease { } func TestProcess(t *testing.T) { + // Create a function to generate a file because Contents can only be read once + testFileFn := func() *gh.ReleaseFile { + return &gh.ReleaseFile{ + Contents: io.NopCloser(strings.NewReader("success")), + Info: gh.ReleaseInfo{ + Tag: nil, + }, + } + } + testCases := []processWorkflowTest{ { name: "successful process", @@ -215,7 +227,7 @@ func TestProcess(t *testing.T) { repoOwner: repoOwner, repoName: repoName, filePattern: filePattern, - fakeFile: io.NopCloser(strings.NewReader("success")), + fakeFile: testFileFn(), err: nil, }, mockParseCfg: &mockParseConfig{ @@ -244,7 +256,7 @@ func TestProcess(t *testing.T) { repoOwner: repoOwner, repoName: repoName, filePattern: filePattern, - fakeFile: io.NopCloser(strings.NewReader("success")), + fakeFile: testFileFn(), err: errTestGetter, }, mockParseCfg: nil, @@ -261,7 +273,7 @@ func TestProcess(t *testing.T) { repoOwner: repoOwner, repoName: repoName, filePattern: filePattern, - fakeFile: io.NopCloser(strings.NewReader("success")), + fakeFile: testFileFn(), err: nil, }, mockParseCfg: &mockParseConfig{ @@ -282,7 +294,7 @@ func TestProcess(t *testing.T) { repoOwner: repoOwner, repoName: repoName, filePattern: filePattern, - fakeFile: io.NopCloser(strings.NewReader("success")), + fakeFile: testFileFn(), err: nil, }, mockParseCfg: &mockParseConfig{ @@ -308,7 +320,7 @@ func TestProcess(t *testing.T) { repoOwner: repoOwner, repoName: repoName, filePattern: filePattern, - fakeFile: io.NopCloser(strings.NewReader("success")), + fakeFile: testFileFn(), err: nil, }, mockParseCfg: &mockParseConfig{ diff --git a/workflows/steps/services/web_feature_consumer/cmd/job/main.go b/workflows/steps/services/web_feature_consumer/cmd/job/main.go index 8d04bed55..65a225837 100644 --- a/workflows/steps/services/web_feature_consumer/cmd/job/main.go +++ b/workflows/steps/services/web_feature_consumer/cmd/job/main.go @@ -92,6 +92,7 @@ func main() { spanneradapters.NewWebFeatureGroupsConsumer(spannerClient), spanneradapters.NewWebFeatureSnapshotsConsumer(spannerClient), data.Parser{}, + data.V3Parser{}, ) // Job Generation diff --git a/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker.go b/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker.go index 8ed504fda..b6f6a8725 100644 --- a/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker.go +++ b/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker.go @@ -16,13 +16,16 @@ package workflow import ( "context" + "errors" "io" "log/slog" "net/http" "time" "github.com/GoogleChrome/webstatus.dev/lib/gen/jsonschema/web_platform_dx__web_features" + "github.com/GoogleChrome/webstatus.dev/lib/gh" "github.com/GoogleChrome/webstatus.dev/lib/webdxfeaturetypes" + "golang.org/x/mod/semver" ) // AssetGetter describes the behavior to get a certain asset from a github release. @@ -31,7 +34,7 @@ type AssetGetter interface { ctx context.Context, owner, repo string, httpClient *http.Client, - filePattern string) (io.ReadCloser, error) + filePattern string) (*gh.ReleaseFile, error) } // AssetParser describes the behavior to parse the io.ReadCloser from AssetGetter into the expected data type. @@ -39,6 +42,11 @@ type AssetParser interface { Parse(io.ReadCloser) (*webdxfeaturetypes.ProcessedWebFeaturesData, error) } +var ( + ErrUnknownAssetVersion = errors.New("unknown asset version") + ErrUnsupportedAssetVersion = errors.New("unsupported asset version") +) + // WebFeatureStorer describes the logic to insert the web features that were returned by the AssetParser. type WebFeatureStorer interface { InsertWebFeatures( @@ -86,25 +94,73 @@ func NewWebFeaturesJobProcessor(assetGetter AssetGetter, metadataStorer WebFeatureMetadataStorer, groupStorer WebDXGroupStorer, snapshotStorer WebDXSnapshotStorer, - webFeaturesDataParser AssetParser, + webFeaturesDataV2Parser AssetParser, + webFeaturesDataV3Parser AssetParser, ) WebFeaturesJobProcessor { return WebFeaturesJobProcessor{ - assetGetter: assetGetter, - storer: storer, - metadataStorer: metadataStorer, - groupStorer: groupStorer, - snapshotStorer: snapshotStorer, - webFeaturesDataParser: webFeaturesDataParser, + assetGetter: assetGetter, + storer: storer, + metadataStorer: metadataStorer, + groupStorer: groupStorer, + snapshotStorer: snapshotStorer, + webFeaturesDataV2Parser: webFeaturesDataV2Parser, + webFeaturesDataV3Parser: webFeaturesDataV3Parser, } } type WebFeaturesJobProcessor struct { - assetGetter AssetGetter - storer WebFeatureStorer - metadataStorer WebFeatureMetadataStorer - groupStorer WebDXGroupStorer - snapshotStorer WebDXSnapshotStorer - webFeaturesDataParser AssetParser + assetGetter AssetGetter + storer WebFeatureStorer + metadataStorer WebFeatureMetadataStorer + groupStorer WebDXGroupStorer + snapshotStorer WebDXSnapshotStorer + webFeaturesDataV2Parser AssetParser + webFeaturesDataV3Parser AssetParser +} + +const ( + // According to https://pkg.go.dev/golang.org/x/mod/semver, the version must start with "v". + v2 = "v2.0.0" + v3 = "v3.0.0" + v4 = "v4.0.0" +) + +func (p WebFeaturesJobProcessor) parseByVersion(ctx context.Context, file *gh.ReleaseFile) ( + *webdxfeaturetypes.ProcessedWebFeaturesData, error) { + if file.Info.Tag == nil { + slog.ErrorContext(ctx, "unknown version", "version", "nil") + + return nil, ErrUnknownAssetVersion + } + + if semver.Compare(*file.Info.Tag, v3) == -1 { + // If less than version 3, use default v2 parser + slog.InfoContext(ctx, "using v2 parser", "version", *file.Info.Tag) + data, err := p.webFeaturesDataV2Parser.Parse(file.Contents) + if err != nil { + slog.ErrorContext(ctx, "unable to parse v2 data", "error", err) + + return nil, err + } + + return data, nil + + } else if semver.Compare(*file.Info.Tag, v4) == -1 { + // If version 3, use v3 parser + slog.InfoContext(ctx, "using v3 parser", "version", *file.Info.Tag) + data, err := p.webFeaturesDataV3Parser.Parse(file.Contents) + if err != nil { + slog.ErrorContext(ctx, "unable to parse v3 data", "error", err) + + return nil, err + } + + return data, nil + } + + slog.ErrorContext(ctx, "unsupported version", "version", *file.Info.Tag) + + return nil, ErrUnsupportedAssetVersion } func (p WebFeaturesJobProcessor) Process(ctx context.Context, job JobArguments) error { @@ -120,7 +176,7 @@ func (p WebFeaturesJobProcessor) Process(ctx context.Context, job JobArguments) return err } - data, err := p.webFeaturesDataParser.Parse(file) + data, err := p.parseByVersion(ctx, file) if err != nil { slog.ErrorContext(ctx, "unable to parse data", "error", err) diff --git a/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker_test.go b/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker_test.go index 46c403ba6..65f5ea224 100644 --- a/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker_test.go +++ b/workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/GoogleChrome/webstatus.dev/lib/gen/jsonschema/web_platform_dx__web_features" + "github.com/GoogleChrome/webstatus.dev/lib/gh" "github.com/GoogleChrome/webstatus.dev/lib/webdxfeaturetypes" ) @@ -48,24 +49,25 @@ type mockDownloadFileFromReleaseConfig struct { expectedFileName string expectedOwner string expectedRepo string - returnReadCloser io.ReadCloser + returnFile *gh.ReleaseFile returnError error } func (m *mockAssetGetter) DownloadFileFromRelease( - _ context.Context, owner, repo string, _ *http.Client, filePattern string) (io.ReadCloser, error) { + _ context.Context, owner, repo string, _ *http.Client, filePattern string) (*gh.ReleaseFile, error) { if filePattern != m.mockDownloadFileFromReleaseCfg.expectedFileName || owner != m.mockDownloadFileFromReleaseCfg.expectedOwner || repo != m.mockDownloadFileFromReleaseCfg.expectedRepo { m.t.Error("unexpected input to DownloadFileFromRelease") } - return m.mockDownloadFileFromReleaseCfg.returnReadCloser, m.mockDownloadFileFromReleaseCfg.returnError + return m.mockDownloadFileFromReleaseCfg.returnFile, m.mockDownloadFileFromReleaseCfg.returnError } type mockAssetParser struct { t *testing.T mockParseCfg mockParseConfig + callCount int } type mockParseConfig struct { @@ -75,13 +77,15 @@ type mockParseConfig struct { } func (m *mockAssetParser) Parse(file io.ReadCloser) (*webdxfeaturetypes.ProcessedWebFeaturesData, error) { + m.callCount++ defer file.Close() fileContents, err := io.ReadAll(file) if err != nil { m.t.Errorf("unable to read file") } if string(fileContents) != m.mockParseCfg.expectedFileContents { - m.t.Error("unexpected file contents") + m.t.Errorf("unexpected file contents want: %s, got: %s", + m.mockParseCfg.expectedFileContents, string(fileContents)) } return m.mockParseCfg.returnData, m.mockParseCfg.returnError @@ -258,7 +262,20 @@ const ( testFileName = "file.txt" ) +func testFile(tag *string, contents string) *gh.ReleaseFile { + return &gh.ReleaseFile{ + Contents: io.NopCloser(strings.NewReader(contents)), + Info: gh.ReleaseInfo{ + Tag: tag, + }, + } +} + func TestProcess(t *testing.T) { + // According https://pkg.go.dev/golang.org/x/mod/semver, the version must start with v + testFileFn := func() *gh.ReleaseFile { + return testFile(valuePtr(v2), "hi features") + } // nolint: dupl testCases := []struct { name string @@ -278,7 +295,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -519,7 +536,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: errTestFailToGetAsset, }, mockParseCfg: mockParseConfig{ @@ -558,7 +575,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -597,7 +614,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -701,7 +718,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -834,7 +851,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -1004,7 +1021,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -1213,7 +1230,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -1443,7 +1460,7 @@ func TestProcess(t *testing.T) { expectedOwner: testRepoOwner, expectedRepo: testRepoName, expectedFileName: testFileName, - returnReadCloser: io.NopCloser(strings.NewReader("hi features")), + returnFile: testFileFn(), returnError: nil, }, mockParseCfg: mockParseConfig{ @@ -1688,6 +1705,12 @@ func TestProcess(t *testing.T) { mockParser := &mockAssetParser{ t: t, mockParseCfg: tc.mockParseCfg, + callCount: 0, + } + mockParserV3 := &mockAssetParser{ + t: t, + mockParseCfg: tc.mockParseCfg, + callCount: 0, } mockStorer := &mockWebFeatureStorer{ t: t, @@ -1717,6 +1740,7 @@ func TestProcess(t *testing.T) { mockGroupStorer, mockSnapshotStorer, mockParser, + mockParserV3, ) err := processor.Process(context.TODO(), NewJobArguments( @@ -1732,3 +1756,134 @@ func TestProcess(t *testing.T) { }) } } + +func valuePtr[T any](in T) *T { return &in } + +func TestParseByVersion(t *testing.T) { + testCases := []struct { + name string + file *gh.ReleaseFile + v2Parser *mockAssetParser + expectedV2CallCount int + v3Parser *mockAssetParser + expectedV3CallCount int + expectedError error + }{ + { + name: "missing tag", + file: testFile(nil, ""), + v2Parser: nil, + v3Parser: nil, + expectedError: ErrUnknownAssetVersion, + expectedV2CallCount: 0, + expectedV3CallCount: 0, + }, + { + name: "v2 parses successfully", + file: testFile(valuePtr(v2), ""), + v2Parser: &mockAssetParser{ + t: t, + mockParseCfg: mockParseConfig{ + expectedFileContents: "", + returnError: nil, + returnData: nil, + }, + callCount: 0, + }, + v3Parser: nil, + expectedError: nil, + expectedV2CallCount: 1, + expectedV3CallCount: 0, + }, + { + name: "v2 parses unsuccessfully", + file: testFile(valuePtr(v2), ""), + v2Parser: &mockAssetParser{ + t: t, + mockParseCfg: mockParseConfig{ + expectedFileContents: "", + returnError: errTestCannotParseData, + returnData: nil, + }, + callCount: 0, + }, + v3Parser: nil, + expectedError: errTestCannotParseData, + expectedV2CallCount: 1, + expectedV3CallCount: 0, + }, + { + name: "v3 parses successfully", + file: testFile(valuePtr(v3), ""), + v2Parser: nil, + v3Parser: &mockAssetParser{ + t: t, + mockParseCfg: mockParseConfig{ + expectedFileContents: "", + returnError: nil, + returnData: nil, + }, + callCount: 0, + }, + expectedError: nil, + expectedV2CallCount: 0, + expectedV3CallCount: 1, + }, + { + name: "v3 parses unsuccessfully", + file: testFile(valuePtr(v3), ""), + v2Parser: nil, + v3Parser: &mockAssetParser{ + t: t, + mockParseCfg: mockParseConfig{ + expectedFileContents: "", + returnError: errTestCannotParseData, + returnData: nil, + }, + callCount: 0, + }, + expectedError: errTestCannotParseData, + expectedV2CallCount: 0, + expectedV3CallCount: 1, + }, + { + name: "unsupported tag", + file: testFile(valuePtr("v4.0.0"), ""), + v2Parser: nil, + v3Parser: nil, + expectedError: ErrUnsupportedAssetVersion, + expectedV2CallCount: 0, + expectedV3CallCount: 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + p := WebFeaturesJobProcessor{ + assetGetter: nil, + webFeaturesDataV2Parser: tc.v2Parser, + webFeaturesDataV3Parser: tc.v3Parser, + storer: nil, + metadataStorer: nil, + groupStorer: nil, + snapshotStorer: nil, + } + _, err := p.parseByVersion(t.Context(), tc.file) + if !errors.Is(err, tc.expectedError) { + t.Errorf("Expected error: %v, Got: %v", tc.expectedError, err) + } + if tc.v2Parser != nil && tc.v2Parser.callCount != tc.expectedV2CallCount { + t.Errorf("Expected v2 call count: %d, Got: %d", tc.expectedV2CallCount, tc.v2Parser.callCount) + } + if tc.v2Parser == nil && tc.expectedV2CallCount > 0 { + t.Error("Expected v2 parser to be called") + } + if tc.v3Parser != nil && tc.v3Parser.callCount != tc.expectedV3CallCount { + t.Errorf("Expected v3 call count: %d, Got: %d", tc.expectedV3CallCount, tc.v3Parser.callCount) + } + if tc.v3Parser == nil && tc.expectedV3CallCount > 0 { + t.Error("Expected v3 parser to be called") + } + }) + } +}