From 2274ee7e8f9739947c9007f5094efbd53a70cfca Mon Sep 17 00:00:00 2001 From: James Sumners Date: Wed, 8 May 2024 10:48:38 -0400 Subject: [PATCH] chore: Updated local repo dir processing --- Readme.md | 20 +++++++++++++------- flags.go | 43 +++++++++++++++++++++++++++++-------------- flags_test.go | 15 +++++++++++++++ main.go | 38 +++++++++++++++++++++++++++++++------- main_test.go | 32 ++++++++++++++++++++++++++++++++ types.go | 10 ++++------ 6 files changed, 124 insertions(+), 34 deletions(-) diff --git a/Readme.md b/Readme.md index 6d88a0d..b5ee6d3 100644 --- a/Readme.md +++ b/Readme.md @@ -80,16 +80,22 @@ modules. The following flags are supported: + -n, --no-externals Disable cloning and processing of external repos. An external repo is + one that provides extra functionality to the "newrelic" module. This + allows processing a single repo with --repo-dir. The default, i.e. not + supplying this flag, is to process all known external repos. + -o, --output-format string Specify the format to write the results as. The default is an ASCII table. Possible values: "ascii" or "markdown". - (default "ascii") - -r, --repo-dir string Specify a local directory that contains the node-newrelic repo. - If not provided, the GitHub repository will be cloned to a local temporary - directory and that will be used. - + (default "markdown") + -r, --repo-dir string Specify a local directory that contains a Node.js instrumentation repo. + If not provided, the main agent GitHub repository will be cloned to a + local temporary directory and that will be used. + -t, --test-dir string Specify the test directory to parse the package.json files. - If not provided, it will default to 'test/versioned'. - + If not provided, it will default to 'test/versioned'. This applies to + the repo provided by the --repo-dir flag. + -v, --verbose Enable verbose output. As the data is being loaded and parsed various logs will be written to stderr that should give indicators of what is happening. diff --git a/flags.go b/flags.go index a6dda4c..b5e005e 100644 --- a/flags.go +++ b/flags.go @@ -9,6 +9,7 @@ import ( ) type appFlags struct { + noExternals bool outputFormat *StringEnumValue repoDir string testDir string @@ -31,6 +32,19 @@ func createAndParseFlags(args []string) error { printUsage(fs.FlagUsages()) } + fs.BoolVarP( + &flags.noExternals, + "no-externals", + "n", + false, + heredoc.Doc(` + Disable cloning and processing of external repos. An external repo is + one that provides extra functionality to the "newrelic" module. This + allows processing a single repo with --repo-dir. The default, i.e. not + supplying this flag, is to process all known external repos. + `), + ) + flags.outputFormat = NewStringEnumValue( []string{"ascii", "markdown"}, "markdown", @@ -51,12 +65,24 @@ func createAndParseFlags(args []string) error { "r", "", heredoc.Doc(` - Specify a local directory that contains a Node.js agent repo. - If not provided, the default GitHub repositories will be cloned to a local temporary - directory and that will be used. + Specify a local directory that contains a Node.js instrumentation repo. + If not provided, the main agent GitHub repository will be cloned to a + local temporary directory and that will be used. `), ) + fs.StringVarP( + &flags.testDir, + "test-dir", + "t", + "", + heredoc.Doc(` + Specify the test directory to parse the package.json files. + If not provided, it will default to 'test/versioned'. This applies to + the repo provided by the --repo-dir flag. + `), + ) + fs.BoolVarP( &flags.verbose, "verbose", @@ -69,17 +95,6 @@ func createAndParseFlags(args []string) error { `), ) - fs.StringVarP( - &flags.testDir, - "test-dir", - "t", - "", - heredoc.Doc(` - Specify the test directory to parse the package.json files. - If not provided, it will default to 'test/versioned'. - `), - ) - // TODO: add flags for generating different formats: // 2. docs site // They should also generate new PRs for the respective repos. diff --git a/flags_test.go b/flags_test.go index 4c7a37a..1f4640b 100644 --- a/flags_test.go +++ b/flags_test.go @@ -5,6 +5,21 @@ import ( "testing" ) +func Test_createAndParseFlags(t *testing.T) { + t.Run("defaults are as expected", func(t *testing.T) { + err := createAndParseFlags([]string{"ignored"}) + expected := appFlags{ + noExternals: false, + outputFormat: &StringEnumValue{ + allowed: []string{"ascii", "markdown"}, + value: "markdown", + }, + } + assert.Nil(t, err) + assert.Equal(t, expected, flags) + }) +} + func Test_StringEnumValue(t *testing.T) { t.Run("only allows specified values", func(t *testing.T) { sev := NewStringEnumValue([]string{"foo", "bar"}, "foo") diff --git a/main.go b/main.go index e4d6b61..e05df5f 100644 --- a/main.go +++ b/main.go @@ -23,10 +23,17 @@ import ( ) var agentRepo = nrRepo{url: `https://github.com/newrelic/node-newrelic.git`, branch: `main`, testPath: `test/versioned`} -var apolloRepo = nrRepo{url: `https://github.com/newrelic/newrelic-node-apollo-server-plugin.git`, branch: `main`, testPath: `tests/versioned`} -var nextRepo = nrRepo{url: `https://github.com/newrelic/newrelic-node-nextjs.git`, branch: `main`, testPath: `tests/versioned`} +var externalsRepos = []nrRepo{ + {url: `https://github.com/newrelic/newrelic-node-apollo-server-plugin.git`, branch: `main`, testPath: `tests/versioned`}, + {url: `https://github.com/newrelic/newrelic-node-nextjs.git`, branch: `main`, testPath: `tests/versioned`}, +} -var columHeaders = map[string]string{"Name": `Package Name`, "MinSupportedVersion": `Minimum Supported Version`, "LatestVersion": `Latest Supported Version`, "MinAgentVersion": `Minimum Agent Version*`} +var columHeaders = map[string]string{ + "Name": `Package Name`, + "MinSupportedVersion": `Minimum Supported Version`, + "LatestVersion": `Latest Supported Version`, + "MinAgentVersion": `Minimum Agent Version*`, +} var appFS = afero.NewOsFs() @@ -61,7 +68,11 @@ func run(args []string) error { var testRepo = nrRepo{repoDir: repoDir, testPath: testDir} repos = []nrRepo{testRepo} } else { - repos = []nrRepo{agentRepo, apolloRepo, nextRepo} + repos = []nrRepo{agentRepo} + } + + if flags.noExternals == false { + repos = append(repos, externalsRepos...) } logger.Info("cloning repositories") @@ -84,9 +95,7 @@ func run(args []string) error { data := processVersionedTestDirs(testDirs, logger) logger.Info("data processing complete") - for _, cloneResult := range cloneResults { - os.RemoveAll(cloneResult.Directory) - } + cleanupTempDirs(cloneResults, logger) slices.SortFunc(data, releaseDataSorter) prunedData := pruneData(data) @@ -102,6 +111,19 @@ func run(args []string) error { return nil } +// cleanupTempDirs removes any temporary directories marked for removal that +// were created during cloning. +func cleanupTempDirs(cloneResults []CloneRepoResult, logger *slog.Logger) { + for _, cloneResult := range cloneResults { + if cloneResult.Remove == false { + logger.Debug("not removing directory " + cloneResult.Directory) + continue + } + logger.Debug("removing directory " + cloneResult.Directory) + _ = appFS.RemoveAll(cloneResult.Directory) + } +} + // processVersionedTestDirs iterates through all versioned test directories, // looking for versioned `package.json` files, and processes what it finds // into release data for each found supported module. @@ -291,6 +313,7 @@ func cloneRepo(repo nrRepo, logger *slog.Logger) CloneRepoResult { return CloneRepoResult{ Directory: repo.repoDir, TestDirectory: repo.testPath, + Remove: false, } } @@ -316,6 +339,7 @@ func cloneRepo(repo nrRepo, logger *slog.Logger) CloneRepoResult { return CloneRepoResult{ Directory: repoDir, TestDirectory: repo.testPath, + Remove: true, } } diff --git a/main_test.go b/main_test.go index 0ddd0de..c7db2fb 100644 --- a/main_test.go +++ b/main_test.go @@ -35,6 +35,38 @@ func (er *errorReader) Read([]byte) (int, error) { return 0, errors.New("boom") } +func Test_cleanupTempDirs(t *testing.T) { + origFS := appFS + t.Cleanup(func() { + appFS = origFS + }) + + appFS = afero.NewMemMapFs() + appFS.Mkdir("/foo", os.ModePerm) + appFS.Mkdir("/bar", os.ModePerm) + appFS.Mkdir("/baz", os.ModePerm) + + collector := &logCollector{} + logger := slog.New(slog.NewTextHandler(collector, &slog.HandlerOptions{Level: slog.LevelDebug})) + cloneResults := []CloneRepoResult{ + {Directory: "/foo", Remove: false}, + {Directory: "/bar", Remove: true}, + {Directory: "/baz", Remove: true}, + } + cleanupTempDirs(cloneResults, logger) + assert.Equal(t, 3, len(collector.logs)) + assert.Contains(t, collector.logs[0], "not removing directory /foo") + assert.Contains(t, collector.logs[1], "removing directory /bar") + assert.Contains(t, collector.logs[2], "removing directory /baz") + + exists, _ := afero.Exists(appFS, "/foo") + assert.Equal(t, true, exists) + exists, _ = afero.Exists(appFS, "/bar") + assert.Equal(t, false, exists) + exists, _ = afero.Exists(appFS, "/baz") + assert.Equal(t, false, exists) +} + func Test_buildLogger(t *testing.T) { t.Run("returns debug level logger", func(t *testing.T) { logger := buildLogger(true) diff --git a/types.go b/types.go index 59265b6..d392272 100644 --- a/types.go +++ b/types.go @@ -20,12 +20,6 @@ type dirIterChan struct { err error } -type repoIterChan struct { - repoDir string - testPath string - err error -} - // CloneRepoResult represents the status of Git repository clone operation. type CloneRepoResult struct { // Directory is the path on the file system that contains the cloned @@ -36,6 +30,10 @@ type CloneRepoResult struct { // versioned tests for the repository. TestDirectory string + // Remove indicates if the Directory should be removed after all data + // processing has completed. + Remove bool + // Error indicates if there was some problem during the clone operation. // Should be `nil` for success results. Error error