5 - Adding error handler for invalid YAML by samanthakem · Pull Request #32 · intuit/judo

samanthakem · 2019-10-01T21:05:41Z

Does all of your new or changed code have unit tests?
Is overall unit test statement coverage still above 85%?
Is code style linting passing with npm run lint?
Is the related GitHub issue number included in your branch name? ex branch name: fix/12-some-bug
Is the related GitHub issue number included in your pull request title? example PR title: "12 - Fixes some bug"
Is documentation still up to date after your change?

As right now, the fix is basically adding the parseYamlFile as a first step for every file, the step will verify the current yaml, if it fails I get the exception message and add it as the reason for the test to fail which indicates the line and column from the invalid yaml file, if there is an invalid yaml file, the test will stop right away. Check the outputs below:

====================== OUTPUTS =====================

Single invalid yaml file fails:

[INFO]     Executing run step: parseYamlFile
[ERROR]    FAILED "parseYamlFile": bad indentation of a mapping entry at line 5, column 3:
      - /On branch ZZZ-[0-9]{0,10}\/no-id/
      ^ (1ms)

[SUMMARY]  ============= JUDO TESTS COMPLETE =============
[SUMMARY]  test-examples/wrong-yml.yml
[ERROR]    FAILED: parseYamlFile (0.001s)
[SUMMARY]  Total tests: 1 (0.001s)
[SUMMARY]  Passed:      0
[SUMMARY]  Failed:      1

Single file test succeeds:

[INFO]     Executing run step: helloWorld
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld" (10ms)
[INFO]     Executing run step: helloWorld2
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld2" (4ms)

[SUMMARY]  ============= JUDO TESTS COMPLETE =============
[SUMMARY]  test-examples/hello-world.yml
[SUCCESS]  PASSED: helloWorld (0.01s)
[SUCCESS]  PASSED: helloWorld2 (0.004s)
[SUMMARY]  Total tests: 2 (0.014s)
[SUMMARY]  Passed:      2
[SUMMARY]  Failed:      0

Directory tests fail with both invalid yaml and invalid step output:

[INFO]     Found 7 tests in test-examples/
[INFO]     Executing run step: helloWorld
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld" (10ms)
[INFO]     Executing run step: helloWorld2
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld2" (4ms)
[INFO]     Executing run step: goodbyeWorld
[INFO]     Running prerequisite commands in: /Users/smonteiro/projects/intuit/judo
[OUTPUT]   this is a prerequisite command

[INFO]     Prerequisites complete
[INFO]     Executing: echo "goodbye!"
[STDOUT]   goodbye!

[INFO]     exited: 0
[SUCCESS]  PASSED "goodbyeWorld" (9ms)
[INFO]     Executing run step: helloMarsShouldFail
[INFO]     Executing: echo "blorp!"
[STDOUT]   blorp!

[INFO]     exited: 0
[ERROR]    FAILED "helloMarsShouldFail": Expected output to contain: goodbye! (5ms)
[INFO]     Executing run step: helloWorld
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld" (4ms)
[INFO]     Executing run step: helloWorld
[INFO]     Executing: echo "hi!"
[STDOUT]   hi!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorld" (3ms)
[INFO]     Executing run step: helloWorldAgain
[INFO]     Executing: echo "hi again!"
[STDOUT]   hi again!

[INFO]     exited: 0
[SUCCESS]  PASSED "helloWorldAgain" (3ms)
[INFO]     Executing run step: goodbyeWorldAgain
[INFO]     Executing: echo "bye again!"
[STDOUT]   bye again!

[INFO]     exited: 0
[SUCCESS]  PASSED "goodbyeWorldAgain" (4ms)
[INFO]     Executing run step: timeoutPass
[INFO]     Executing: sleep 1s
[INFO]     exited: 0
[SUCCESS]  PASSED "timeoutPass" (1004ms)
[INFO]     Executing run step: timeoutFail
[INFO]     Executing: sleep 2s
[INFO]     exited: 0
[SUCCESS]  PASSED "timeoutFail" (2005ms)
[INFO]     Executing run step: timeoutOngoing
[INFO]     Executing: bash ./test-examples/timeout-suite/timeout-test.sh
[STDOUT]   waiting 1s

[STDOUT]   first sleep done, waiting 1s

[STDOUT]   second sleep done, waiting 3s

[INFO]     exited: 0
[SUCCESS]  PASSED "timeoutOngoing" (5026ms)
[INFO]     Executing run step: parseYamlFile
[ERROR]    FAILED "parseYamlFile": bad indentation of a mapping entry at line 5, column 3:
      - /On branch ZZZ-[0-9]{0,10}\/no-id/
      ^ (1ms)

[SUMMARY]  ============= JUDO TESTS COMPLETE =============
[SUMMARY]  test-examples/hello-world.yml
[SUCCESS]  PASSED: helloWorld (0.01s)
[SUCCESS]  PASSED: helloWorld2 (0.004s)
[SUMMARY]  test-examples/simple-test-suite/goodbye-world-fail.yml
[SUCCESS]  PASSED: goodbyeWorld (0.009s)
[ERROR]    FAILED: helloMarsShouldFail (0.005s)
[SUMMARY]  test-examples/simple-test-suite/hello-world-regex.yml
[SUCCESS]  PASSED: helloWorld (0.004s)
[SUMMARY]  test-examples/simple-test-suite/hello-world.yml
[SUCCESS]  PASSED: helloWorld (0.003s)
[SUMMARY]  test-examples/simple-test-suite/nested-test-suite/hello-goodbye-world-again.yml
[SUCCESS]  PASSED: helloWorldAgain (0.003s)
[SUCCESS]  PASSED: goodbyeWorldAgain (0.004s)
[SUMMARY]  test-examples/timeout-suite/timeout.yml
[SUCCESS]  PASSED: timeoutPass (1.004s)
[SUCCESS]  PASSED: timeoutFail (2.005s)
[SUCCESS]  PASSED: timeoutOngoing (5.026s)
[SUMMARY]  test-examples/wrong-yml.yml
[ERROR]    FAILED: parseYamlFile (0.001s)
[SUMMARY]  Total tests: 12 (8.078s)
[SUMMARY]  Passed:      10
[SUMMARY]  Failed:      2

worldjoe · 2019-10-01T21:32:13Z

+    runSteps = yamlFile.run;
+    runStepNames = Object.keys(runSteps);
+  } catch (e) {
+    parseErrorMessage = e.message;


If there is an error loading the file is there any benefit in continuing? Wouldn't the first execute run test throw an error? which i see you catch below, but it might make more sense to just bail here.

I was mostly thinking about the user experience, I wanted to still show the summary for the tests, wanted to give a stepName to this step of checking yml file as well instead of just bail over there.

If I just bail there, I would replace parseErrorMessage = e.message; by

logger.error(e.message); process.exit(1);

the output for first approach is:

[INFO] Executing run step: parseYamlFile [ERROR] FAILED "parseYamlFile": bad indentation of a mapping entry at line 5, column 3: - /On branch ZZZ-[0-9]{0,10}\/no-id/ ^ (1ms) [SUMMARY] ============= JUDO TESTS COMPLETE ============= [SUMMARY] test-examples/wrong-yml.yml [ERROR] FAILED: parseYamlFile (0.001s) [SUMMARY] Total tests: 1 (0.001s) [SUMMARY] Passed: 0 [SUMMARY] Failed: 1

and for the second approach is:

[ERROR] bad indentation of a mapping entry at line 5, column 3: - /On branch ZZZ-[0-9]{0,10}\/no-id/ ^

@worldjoe do you think I should use this second approach then? Thinking about the user, I would prefer the first but it is up to you.

ejfrancis · 2019-10-01T21:56:55Z

Adding a special named step seems like a bit of a hack. Getting the line number and preview here is great, but from the user's perspective I'd want the output to be something unique to when parse errors occur to make it more clear

samanthakem · 2019-10-02T17:33:42Z

Okay so I just added a logger.error(...) which is specifies the issue with yaml parsing + process.exit(1).

The output would be this for a invalid yaml file:

[ERROR]   YAML PARSER FAILED on file test-examples/wrong.yml: bad indentation of a mapping entry at line 5, column 3:
      - /On branch ZZZ-[0-9]{0,10}\/no-id/
      ^

ejfrancis

ran it locally and everything looks good. thanks for the contribution!

samanthakem added 2 commits October 1, 2019 13:48

Adding fix for unhandled yaml parsing

e343720

Adding unit test and lint fix

7d9dd75

samanthakem mentioned this pull request Oct 1, 2019

handle if invalid YAML passed in #5

Closed

worldjoe reviewed Oct 1, 2019

View reviewed changes

samanthakem added 2 commits October 2, 2019 10:30

Add specific yaml parser handler

2e6a57b

Remove wrong files that have been checking in by mistake

af1fdeb

Rename logger content

df7aad2

samanthakem requested a review from worldjoe October 2, 2019 17:41

ejfrancis approved these changes Oct 2, 2019

View reviewed changes

ejfrancis added hacktoberfest minor create a minor release release create a release enhancement New feature or request and removed release create a release labels Oct 2, 2019

ejfrancis merged commit e819f36 into intuit:master Oct 2, 2019

Navigation Menu

Uh oh!

5 - Adding error handler for invalid YAML - #32

5 - Adding error handler for invalid YAML#32
ejfrancis merged 5 commits into
intuit:masterfrom
samanthakem:fix/5-handleInvalidYaml

samanthakem commented Oct 1, 2019

Uh oh!

worldjoe Oct 1, 2019

Uh oh!

samanthakem Oct 1, 2019 •

edited

Loading

Uh oh!

ejfrancis commented Oct 1, 2019

Uh oh!

samanthakem commented Oct 2, 2019 •

edited

Loading

Uh oh!

ejfrancis left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

samanthakem commented Oct 1, 2019

Uh oh!

worldjoe Oct 1, 2019

Choose a reason for hiding this comment

Uh oh!

samanthakem Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ejfrancis commented Oct 1, 2019

Uh oh!

samanthakem commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ejfrancis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samanthakem Oct 1, 2019 •

edited

Loading

samanthakem commented Oct 2, 2019 •

edited

Loading