From 071f835abbe550066fb2940cb49ef8502535d97d Mon Sep 17 00:00:00 2001 From: james-stocks Date: Thu, 9 Mar 2017 11:03:18 +0000 Subject: [PATCH 1/4] Update tests to expect specific output Test cases that expect metadata-json-lint to return non-zero should also test that the output from metadata-json-lint matches some expected output. Otherwise the test will pass if _any_ problem occurs. --- .gitignore | 1 + tests/bad_license/expected | 1 + tests/broken/expected | 1 + tests/duplicate-dep/expected | 1 + tests/long_summary/expected | 1 + tests/missing_version_requirement/expected | 1 + tests/mixed_version_syntax/expected | 1 + tests/multiple_problems/expected | 3 +++ tests/no_files/expected | 1 + tests/noname/expected | 1 + tests/open_ended_dependency/expected | 1 + tests/rake_global_options/expected | 1 + tests/tags_no_array/expected | 1 + tests/test.sh | 24 +++++++++++++++++++--- tests/types/expected | 1 + 15 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests/bad_license/expected create mode 100644 tests/broken/expected create mode 100644 tests/duplicate-dep/expected create mode 100644 tests/long_summary/expected create mode 100644 tests/missing_version_requirement/expected create mode 100644 tests/mixed_version_syntax/expected create mode 100644 tests/multiple_problems/expected create mode 100644 tests/no_files/expected create mode 100644 tests/noname/expected create mode 100644 tests/open_ended_dependency/expected create mode 100644 tests/rake_global_options/expected create mode 100644 tests/tags_no_array/expected create mode 100644 tests/types/expected diff --git a/.gitignore b/.gitignore index 32e85b1..3defcdc 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ Gemfile.lock /.bundle /vendor +tests/*/last_output diff --git a/tests/bad_license/expected b/tests/bad_license/expected new file mode 100644 index 0000000..c5b2481 --- /dev/null +++ b/tests/bad_license/expected @@ -0,0 +1 @@ +Warning: License identifier Unknown-1.0 is not in the SPDX list: http://spdx.org/licenses/ diff --git a/tests/broken/expected b/tests/broken/expected new file mode 100644 index 0000000..a66f1bf --- /dev/null +++ b/tests/broken/expected @@ -0,0 +1 @@ +Error: Unable to parse metadata.json: 743: unexpected token at diff --git a/tests/duplicate-dep/expected b/tests/duplicate-dep/expected new file mode 100644 index 0000000..3d98231 --- /dev/null +++ b/tests/duplicate-dep/expected @@ -0,0 +1 @@ +Error: duplicate dependencies on puppetlabs/stdlib diff --git a/tests/long_summary/expected b/tests/long_summary/expected new file mode 100644 index 0000000..42348c5 --- /dev/null +++ b/tests/long_summary/expected @@ -0,0 +1 @@ +Error: summary exceeds 144 characters in metadata.json diff --git a/tests/missing_version_requirement/expected b/tests/missing_version_requirement/expected new file mode 100644 index 0000000..5641809 --- /dev/null +++ b/tests/missing_version_requirement/expected @@ -0,0 +1 @@ +Warning: Dependency puppetlabs/stdlib has an open ended dependency version requirement diff --git a/tests/mixed_version_syntax/expected b/tests/mixed_version_syntax/expected new file mode 100644 index 0000000..818d311 --- /dev/null +++ b/tests/mixed_version_syntax/expected @@ -0,0 +1 @@ +Invalid 'version_requirement' field in metadata.json: Unparsable version range: ">= 3.2.x" diff --git a/tests/multiple_problems/expected b/tests/multiple_problems/expected new file mode 100644 index 0000000..ac27ab7 --- /dev/null +++ b/tests/multiple_problems/expected @@ -0,0 +1,3 @@ +Error: Required field 'license' not found in metadata.json. +Error: Required field 'summary' not found in metadata.json. +Error: Deprecated field 'types' found in metadata.json. diff --git a/tests/no_files/expected b/tests/no_files/expected new file mode 100644 index 0000000..0c29e37 --- /dev/null +++ b/tests/no_files/expected @@ -0,0 +1 @@ +Error: Unable to read metadata file: No such file or directory diff --git a/tests/noname/expected b/tests/noname/expected new file mode 100644 index 0000000..f4735ac --- /dev/null +++ b/tests/noname/expected @@ -0,0 +1 @@ +Error: Required field 'name' not found in metadata.json. diff --git a/tests/open_ended_dependency/expected b/tests/open_ended_dependency/expected new file mode 100644 index 0000000..d0eac73 --- /dev/null +++ b/tests/open_ended_dependency/expected @@ -0,0 +1 @@ +Warning: Dependency puppetlabs/stdlib has an open ended dependency version requirement >= 3.2.0 diff --git a/tests/rake_global_options/expected b/tests/rake_global_options/expected new file mode 100644 index 0000000..5ba71a5 --- /dev/null +++ b/tests/rake_global_options/expected @@ -0,0 +1 @@ +Warning: License identifier invalid_license is not in the SPDX list: http://spdx.org/licenses/ diff --git a/tests/tags_no_array/expected b/tests/tags_no_array/expected new file mode 100644 index 0000000..52325f4 --- /dev/null +++ b/tests/tags_no_array/expected @@ -0,0 +1 @@ +Warning: Tags must be in an array. Currently it's a String. diff --git a/tests/test.sh b/tests/test.sh index 0e841f7..d0727bc 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -31,12 +31,28 @@ test_bin() { local expect=$1; shift local RESULT=-1 cd $name; - bundle exec metadata-json-lint $* metadata.json >/dev/null 2>&1 + bundle exec metadata-json-lint $* metadata.json >last_output 2>&1 RESULT=$? if [ $RESULT -ne $expect ]; then - fail "Failing Test '${name}' (bin)" + fail "Failing Test (unexpected exit code) '${name}' (bin)" else - echo "Successful Test '${name}' (bin)" + # If the test is not expected to succeed then it should match an expected output + if [ $expect -eq $SUCCESS ]; then + echo "Successful Test '${name}' (bin)" + else + if [ -f expected ]; then + if grep --quiet "`cat expected`" last_output; then + echo "Successful Test '${name}' (bin)" + else + fail "Failing Test (did not get expected output) '${name}' (bin)" + echo "Expected: '`cat expected`'" + echo "Actual: '`cat last_output`'" + fi + else + fail "Failing Test (expected output file ${name}/expected is missing) '${name}' (bin)" + echo "Actual output that needs tested: '`cat last_output`'" + fi + fi fi cd .. } @@ -125,6 +141,8 @@ fi cd .. # Test changing the rake task using settings +# The bin test will fail due to strict licensing +# The rake test should pass due to licensing option being set in Rakefile test_bin "rake_global_options" $FAILURE test_rake "rake_global_options" $SUCCESS diff --git a/tests/types/expected b/tests/types/expected new file mode 100644 index 0000000..d36eef1 --- /dev/null +++ b/tests/types/expected @@ -0,0 +1 @@ +Error: Deprecated field 'types' found in metadata.json. From 942ad1b4f33de863db993e93c9b5d92ca5edb03c Mon Sep 17 00:00:00 2001 From: james-stocks Date: Thu, 9 Mar 2017 11:50:35 +0000 Subject: [PATCH 2/4] Rescue exception if metadata.json does not exist to read If metadata.json does not exist; output a clear message instead of having an unhandled exception. --- lib/metadata_json_lint.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/metadata_json_lint.rb b/lib/metadata_json_lint.rb index 1a98d51..31180e4 100644 --- a/lib/metadata_json_lint.rb +++ b/lib/metadata_json_lint.rb @@ -53,7 +53,11 @@ def parse(metadata) options = options().clone # Configuration from rake tasks yield options if block_given? - f = File.read(metadata) + begin + f = File.read(metadata) + rescue Exception => e + abort("Error: Unable to read metadata file: #{e.exception}") + end begin parsed = JSON.parse(f) From 82d07463a3f3d9241a18718afbfc32a1721497c9 Mon Sep 17 00:00:00 2001 From: james-stocks Date: Fri, 10 Mar 2017 14:52:48 +0000 Subject: [PATCH 3/4] Fix json for long summary test This test was failing due to invalid JSON syntax and not due to a long summary. --- tests/long_summary/metadata.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/long_summary/metadata.json b/tests/long_summary/metadata.json index 2c8903b..52c240d 100644 --- a/tests/long_summary/metadata.json +++ b/tests/long_summary/metadata.json @@ -2,11 +2,11 @@ "name": "foo-bar", "version": "1.0.0", "author": "foo/bar", - "summary": "This is obviously a way way tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo long" + "summary": "This is obviously a way way tooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo long", "license": "Apache-2.0", - "source": "" - "project_page": "" - "issues_url": "" + "source": "", + "project_page": "", + "issues_url": "", "operatingsystem_support": [ { "operatingsystem": "RedHat", From 8fd81981204ab6d87b8bd74842a7c2612b988a5f Mon Sep 17 00:00:00 2001 From: james-stocks Date: Mon, 13 Mar 2017 17:09:31 +0000 Subject: [PATCH 4/4] Improve printed output for failing tests Point the user to the ${test_name}/last_output file in the event of a failure. Improve the output when a test fails to be more readable. --- tests/test.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test.sh b/tests/test.sh index d0727bc..a6b1157 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -34,7 +34,8 @@ test_bin() { bundle exec metadata-json-lint $* metadata.json >last_output 2>&1 RESULT=$? if [ $RESULT -ne $expect ]; then - fail "Failing Test (unexpected exit code) '${name}' (bin)" + fail "Failing Test '${name}' (unexpected exit code '${RESULT}' instead of '${expect}') (bin)" + echo " Note: you can examine '${name}/last_output' for any output" else # If the test is not expected to succeed then it should match an expected output if [ $expect -eq $SUCCESS ]; then @@ -44,13 +45,14 @@ test_bin() { if grep --quiet "`cat expected`" last_output; then echo "Successful Test '${name}' (bin)" else - fail "Failing Test (did not get expected output) '${name}' (bin)" - echo "Expected: '`cat expected`'" - echo "Actual: '`cat last_output`'" + fail "Failing Test '${name}' (did not get expected output) (bin)" + echo " Comparing '${name}/expected' with '${name}/last_output':" + echo " Expected: '`cat expected`'" + echo " Actual: '`cat last_output`'" fi else - fail "Failing Test (expected output file ${name}/expected is missing) '${name}' (bin)" - echo "Actual output that needs tested: '`cat last_output`'" + fail "Failing Test '${name}' (expected output file ${name}/expected is missing) (bin)" + echo " Actual output that needs tested ('${name}/last_output'): '`cat last_output`'" fi fi fi