Skip to content

Commit

Permalink
Adjust npmExecuteLint (output-format, print output to console) (#4407)
Browse files Browse the repository at this point in the history
* Adjust npmExecuteLint (output-format, print output to console)

Co-authored-by: Srinikitha Kondreddy <[email protected]>
  • Loading branch information
marcusholl and srinikitha09 authored Jun 19, 2023
1 parent 799853e commit 8b36ae7
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 11 deletions.
37 changes: 33 additions & 4 deletions cmd/npmExecuteLint.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ func runNpmExecuteLint(npmExecutor npm.Executor, utils lintUtils, config *npmExe
}
}

err := runDefaultLint(npmExecutor, utils, config.FailOnError)
err := runDefaultLint(npmExecutor, utils, config.FailOnError, config.OutputFormat, config.OutputFileName)

if err != nil {
return err
}
Expand All @@ -127,7 +128,7 @@ func runLintScript(npmExecutor npm.Executor, runScript string, failOnError bool)
return nil
}

func runDefaultLint(npmExecutor npm.Executor, utils lintUtils, failOnError bool) error {
func runDefaultLint(npmExecutor npm.Executor, utils lintUtils, failOnError bool, outputFormat string, outputFileName string) error {
execRunner := utils.getExecRunner()
eslintConfigs := findEslintConfigs(utils)

Expand All @@ -145,7 +146,16 @@ func runDefaultLint(npmExecutor npm.Executor, utils lintUtils, failOnError bool)
if dir != "." {
lintPattern = dir + "/**/*.js"
}
err = execRunner.RunExecutable("npx", "eslint", lintPattern, "-f", "checkstyle", "-o", "./"+strconv.Itoa(i)+"_defaultlint.xml", "--ignore-pattern", "node_modules/", "--ignore-pattern", ".eslintrc.js")

args := prepareArgs([]string{
"eslint",
lintPattern,
"-f", outputFormat,
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
}, fmt.Sprintf("./%s_%%s", strconv.Itoa(i)), outputFileName)

err = execRunner.RunExecutable("npx", args...)
if err != nil {
if failOnError {
return fmt.Errorf("Lint execution failed. This might be the result of severe linting findings, problems with the provided ESLint configuration (%s), or another issue. Please examine the linting results in the UI or in %s, if available, or the log above. ", config, strconv.Itoa(i)+"_defaultlint.xml")
Expand All @@ -160,7 +170,18 @@ func runDefaultLint(npmExecutor npm.Executor, utils lintUtils, failOnError bool)

// Ignore possible errors when invoking ESLint to not fail the pipeline based on linting results
_ = execRunner.RunExecutable("npm", "install", "eslint@^7.0.0", "typescript@^3.7.4", "@typescript-eslint/parser@^3.0.0", "@typescript-eslint/eslint-plugin@^3.0.0")
_ = execRunner.RunExecutable("npx", "--no-install", "eslint", ".", "--ext", ".js,.jsx,.ts,.tsx", "-c", ".pipeline/.eslintrc.json", "-f", "checkstyle", "-o", "./defaultlint.xml", "--ignore-pattern", ".eslintrc.js")

args := prepareArgs([]string{
"--no-install",
"eslint",
".",
"--ext", ".js,.jsx,.ts,.tsx",
"-c", ".pipeline/.eslintrc.json",
"-f", outputFormat,
"--ignore-pattern", ".eslintrc.js",
}, "./%s", outputFileName)

_ = execRunner.RunExecutable("npx", args...)
}
return nil
}
Expand All @@ -186,3 +207,11 @@ func findEslintConfigs(utils lintUtils) []string {
}
return eslintConfigs
}

func prepareArgs(defaultArgs []string, outputFileNamePattern, outputFileName string) []string {
if outputFileName != "" { // in this case we omit the -o flag and output will go to the log
defaultArgs = append(defaultArgs, "-o", fmt.Sprintf(outputFileNamePattern, outputFileName))
}
return defaultArgs

}
22 changes: 22 additions & 0 deletions cmd/npmExecuteLint_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

186 changes: 179 additions & 7 deletions cmd/npmExecuteLint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func newLintMockUtilsBundle() mockLintUtilsBundle {
}

func TestNpmExecuteLint(t *testing.T) {
defaultConfig := npmExecuteLintOptions{RunScript: "ci-lint"}
defaultConfig := npmExecuteLintOptions{RunScript: "ci-lint", OutputFormat: "checkstyle", OutputFileName: "defaultlint.xml"}

t.Run("Call with ci-lint script and one package.json", func(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
Expand Down Expand Up @@ -67,7 +67,42 @@ func TestNpmExecuteLint(t *testing.T) {

if assert.NoError(t, err) {
if assert.Equal(t, 2, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{"eslint", ".", "-f", "checkstyle", "-o", "./0_defaultlint.xml", "--ignore-pattern", "node_modules/", "--ignore-pattern", ".eslintrc.js"}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "checkstyle",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
"-o", "./0_defaultlint.xml"}}, lintUtils.execRunner.Calls[1])
}
}
})

t.Run("Call default with ESLint config from user - no redirect to file, stylish format", func(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
lintUtils.AddFile("package.json", []byte("{\"name\": \"Test\" }"))
lintUtils.AddFile(".eslintrc.json", []byte("{\"name\": \"Test\" }"))

config := npmExecuteLintOptions{RunScript: "ci-lint", OutputFormat: "stylish", OutputFileName: ""}
config.DefaultNpmRegistry = "foo.bar"

npmUtils := newNpmMockUtilsBundle()
npmUtils.execRunner = lintUtils.execRunner
npmExecutor := npm.Execute{Utils: &npmUtils, Options: npm.ExecutorOptions{}}

err := runNpmExecuteLint(&npmExecutor, &lintUtils, &config)

if assert.NoError(t, err) {
if assert.Equal(t, 2, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "stylish",
"--ignore-pattern",
"node_modules/",
"--ignore-pattern", ".eslintrc.js",
// no -o, --output-file in this case.
}}, lintUtils.execRunner.Calls[1])
}
}
})
Expand All @@ -89,8 +124,61 @@ func TestNpmExecuteLint(t *testing.T) {

if assert.NoError(t, err) {
if assert.Equal(t, 3, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{"eslint", ".", "-f", "checkstyle", "-o", "./0_defaultlint.xml", "--ignore-pattern", "node_modules/", "--ignore-pattern", ".eslintrc.js"}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{"eslint", "src/**/*.js", "-f", "checkstyle", "-o", "./1_defaultlint.xml", "--ignore-pattern", "node_modules/", "--ignore-pattern", ".eslintrc.js"}}, lintUtils.execRunner.Calls[2])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "checkstyle",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
"-o", "./0_defaultlint.xml",
}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
"src/**/*.js",
"-f", "checkstyle",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
"-o", "./1_defaultlint.xml",
}}, lintUtils.execRunner.Calls[2])
}
}
})

t.Run("Call default with two ESLint configs from user - no redirect to file, stylish format", func(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
lintUtils.AddFile("package.json", []byte("{\"name\": \"Test\" }"))
lintUtils.AddFile(".eslintrc.json", []byte("{\"name\": \"Test\" }"))
lintUtils.AddFile(filepath.Join("src", ".eslintrc.json"), []byte("{\"name\": \"Test\" }"))

config := defaultConfig
config.DefaultNpmRegistry = "foo.bar"
config.OutputFormat = "stylish"
config.OutputFileName = ""

npmUtils := newNpmMockUtilsBundle()
npmUtils.execRunner = lintUtils.execRunner
npmExecutor := npm.Execute{Utils: &npmUtils, Options: npm.ExecutorOptions{}}

err := runNpmExecuteLint(&npmExecutor, &lintUtils, &config)

if assert.NoError(t, err) {
if assert.Equal(t, 3, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "stylish",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
// no -o --output-file in this case.
}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
"src/**/*.js",
"-f", "stylish",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
// no -o --output-file in this case.
}}, lintUtils.execRunner.Calls[2])
}
}
})
Expand All @@ -111,7 +199,50 @@ func TestNpmExecuteLint(t *testing.T) {
if assert.NoError(t, err) {
if assert.Equal(t, 3, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npm", Params: []string{"install", "eslint@^7.0.0", "typescript@^3.7.4", "@typescript-eslint/parser@^3.0.0", "@typescript-eslint/eslint-plugin@^3.0.0"}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{"--no-install", "eslint", ".", "--ext", ".js,.jsx,.ts,.tsx", "-c", ".pipeline/.eslintrc.json", "-f", "checkstyle", "-o", "./defaultlint.xml", "--ignore-pattern", ".eslintrc.js"}}, lintUtils.execRunner.Calls[2])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"--no-install",
"eslint",
".",
"--ext",
".js,.jsx,.ts,.tsx",
"-c", ".pipeline/.eslintrc.json",
"-f", "checkstyle",
"--ignore-pattern", ".eslintrc.js",
"-o", "./defaultlint.xml",
}}, lintUtils.execRunner.Calls[2])
}
}
})

t.Run("Default without ESLint config - no redirect to file, stylish format", func(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
lintUtils.AddFile("package.json", []byte("{\"name\": \"Test\" }"))

config := defaultConfig
config.DefaultNpmRegistry = "foo.bar"
config.OutputFormat = "stylish"
config.OutputFileName = ""

npmUtils := newNpmMockUtilsBundle()
npmUtils.execRunner = lintUtils.execRunner
npmExecutor := npm.Execute{Utils: &npmUtils, Options: npm.ExecutorOptions{}}

err := runNpmExecuteLint(&npmExecutor, &lintUtils, &config)

if assert.NoError(t, err) {
if assert.Equal(t, 3, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npm", Params: []string{"install", "eslint@^7.0.0", "typescript@^3.7.4", "@typescript-eslint/parser@^3.0.0", "@typescript-eslint/eslint-plugin@^3.0.0"}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"--no-install",
"eslint",
".",
"--ext",
".js,.jsx,.ts,.tsx",
"-c", ".pipeline/.eslintrc.json",
"-f", "stylish",
"--ignore-pattern", ".eslintrc.js",
// no -o --output-file in this case.
}}, lintUtils.execRunner.Calls[2])
}
}
})
Expand Down Expand Up @@ -143,10 +274,44 @@ func TestNpmExecuteLint(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
lintUtils.AddFile("package.json", []byte("{\"name\": \"Test\" }"))
lintUtils.AddFile(".eslintrc.json", []byte("{\"name\": \"Test\" }"))
lintUtils.execRunner = &mock.ExecMockRunner{ShouldFailOnCommand: map[string]error{"eslint . -f checkstyle -o ./0_defaultlint.xml --ignore-pattern node_modules/ --ignore-pattern .eslintrc.js": errors.New("exit 1")}}
lintUtils.execRunner = &mock.ExecMockRunner{ShouldFailOnCommand: map[string]error{
"eslint . -f checkstyle --ignore-pattern node_modules/ --ignore-pattern .eslintrc.js -o ./0_defaultlint.xml": errors.New("exit 1")}}

config := defaultConfig
config.FailOnError = true
config.DefaultNpmRegistry = "foo.bar"

npmUtils := newNpmMockUtilsBundle()
npmUtils.execRunner = lintUtils.execRunner
npmExecutor := npm.Execute{Utils: &npmUtils, Options: npm.ExecutorOptions{}}

err := runNpmExecuteLint(&npmExecutor, &lintUtils, &config)

if assert.EqualError(t, err, "Lint execution failed. This might be the result of severe linting findings, problems with the provided ESLint configuration (.eslintrc.json), or another issue. Please examine the linting results in the UI or in 0_defaultlint.xml, if available, or the log above. ") {
if assert.Equal(t, 2, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "checkstyle",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
"-o", "./0_defaultlint.xml",
}}, lintUtils.execRunner.Calls[1])
}
}
})

t.Run("Call default with ESLint config from user and failOnError - no redirect to file, stylish format", func(t *testing.T) {
lintUtils := newLintMockUtilsBundle()
lintUtils.AddFile("package.json", []byte("{\"name\": \"Test\" }"))
lintUtils.AddFile(".eslintrc.json", []byte("{\"name\": \"Test\" }"))
lintUtils.execRunner = &mock.ExecMockRunner{ShouldFailOnCommand: map[string]error{
"eslint . -f stylish --ignore-pattern node_modules/ --ignore-pattern .eslintrc.js": errors.New("exit 1")}}

config := defaultConfig
config.FailOnError = true
config.OutputFormat = "stylish"
config.OutputFileName = ""
config.DefaultNpmRegistry = "foo.bar"

npmUtils := newNpmMockUtilsBundle()
Expand All @@ -157,7 +322,14 @@ func TestNpmExecuteLint(t *testing.T) {

if assert.EqualError(t, err, "Lint execution failed. This might be the result of severe linting findings, problems with the provided ESLint configuration (.eslintrc.json), or another issue. Please examine the linting results in the UI or in 0_defaultlint.xml, if available, or the log above. ") {
if assert.Equal(t, 2, len(lintUtils.execRunner.Calls)) {
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{"eslint", ".", "-f", "checkstyle", "-o", "./0_defaultlint.xml", "--ignore-pattern", "node_modules/", "--ignore-pattern", ".eslintrc.js"}}, lintUtils.execRunner.Calls[1])
assert.Equal(t, mock.ExecCall{Exec: "npx", Params: []string{
"eslint",
".",
"-f", "stylish",
"--ignore-pattern", "node_modules/",
"--ignore-pattern", ".eslintrc.js",
// no -o, --output-file in this case.
}}, lintUtils.execRunner.Calls[1])
}
}
})
Expand Down
24 changes: 24 additions & 0 deletions resources/metadata/npmExecuteLint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,30 @@ spec:
mandatory: false
aliases:
- name: npm/defaultNpmRegistry
- name: outputFormat
type: string
description: eslint output format, e.g. stylish, checkstyle
scope:
- PARAMETERS
- GENERAL
- STAGES
- STEPS
mandatory: false
default: checkstyle
aliases:
- name: npm/outputFormat
- name: outputFileName
type: string
description: name of the output file. There might be a 'N_' prefix where 'N' is a number. When the empty string is provided, we will print to console
scope:
- PARAMETERS
- GENERAL
- STAGES
- STEPS
mandatory: false
default: defaultlint.xml
aliases:
- name: npm/outputFormat
containers:
- name: node
image: node:lts-buster

0 comments on commit 8b36ae7

Please sign in to comment.