From 1a9b340cb9645c528769249d13b66bdc6d9ce630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sat, 19 Aug 2017 18:24:21 +0200 Subject: [PATCH 1/5] Use a Map instead of an Obejct to store tests This change has two reasons: - The object was actually used as a map - Using an object leads some problems with the private_class_fields/constructor.js test, since `tests[test_name] || {}` returned the Obejct constructor instead of an empty object. --- scripts/run_flow_tests.js | 40 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/scripts/run_flow_tests.js b/scripts/run_flow_tests.js index 00cb980c81..a88d20aa85 100644 --- a/scripts/run_flow_tests.js +++ b/scripts/run_flow_tests.js @@ -5,6 +5,15 @@ const fs = require("fs"); const chalk = require("chalk"); const parse = require("..").parse; +function map_get_default(map, key, defaultConstructor) { + if (map.has(key)) { + return map.get(key); + } + const value = new defaultConstructor(); + map.set(key, value); + return value; +} + function list_files(root, dir) { const files = fs.readdirSync(dir ? path.join(root, dir) : root); let result = []; @@ -22,7 +31,7 @@ function list_files(root, dir) { function get_tests(root_dir) { const files = list_files(root_dir); - const tests = {}; + const tests = new Map(); for (let i = 0; i < files.length; i++) { const file = files[i]; const test_name = path.dirname(file); @@ -34,8 +43,8 @@ function get_tests(root_dir) { continue; } - const cases = (tests[test_name] = tests[test_name] || {}); - const case_ = (cases[case_name] = cases[case_name] || {}); + const cases = map_get_default(tests, test_name, Map); + const case_ = map_get_default(cases, case_name, Object); const content = fs.readFileSync(path.join(root_dir, file), { encoding: "utf8", @@ -60,20 +69,11 @@ function get_hardcoded_tests() { const tests = get_tests( path.join(__dirname, "../build/flow/src/parser/test/flow") ); - const result = {}; - for (const section in tests) { - if (tests.hasOwnProperty(section)) { - const test = tests[section]; - const cases = []; - // TODO: use Object.values if we require new enough node - for (const case_ in test) { - if (test.hasOwnProperty(case_)) { - cases.push(test[case_]); - } - } - result[section] = { tests: cases }; - } - } + const result = new Map(); + tests.forEach((section, sectionName) => { + const cases = Array.from(section.values()); + result.set(sectionName, { tests: cases }); + }); return result; } @@ -92,11 +92,11 @@ const flowOptionsMapping = { let failedTests = 0; let successTests = 0; -const hardcodedTests = get_hardcoded_tests(); -Object.keys(hardcodedTests).forEach(sectionName => { +const tests = get_hardcoded_tests(); +tests.forEach((section, sectionName) => { console.log(""); console.log(`### ${sectionName} ###`); - hardcodedTests[sectionName].tests.forEach(test => { + section.tests.forEach(test => { const shouldSuccess = test.expected_ast && (!Array.isArray(test.expected_ast.errors) || From 942d22dd7013daa9a0d4489c65019f29f4a0a787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sat, 19 Aug 2017 18:25:18 +0200 Subject: [PATCH 2/5] Remove the `get_harcoded_tests` function, use `get_tests` --- scripts/run_flow_tests.js | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/scripts/run_flow_tests.js b/scripts/run_flow_tests.js index a88d20aa85..f74df55c62 100644 --- a/scripts/run_flow_tests.js +++ b/scripts/run_flow_tests.js @@ -5,6 +5,8 @@ const fs = require("fs"); const chalk = require("chalk"); const parse = require("..").parse; +const TESTS_FOLDER = path.join(__dirname, "../build/flow/src/parser/test/flow"); + function map_get_default(map, key, defaultConstructor) { if (map.has(key)) { return map.get(key); @@ -65,18 +67,6 @@ function get_tests(root_dir) { return tests; } -function get_hardcoded_tests() { - const tests = get_tests( - path.join(__dirname, "../build/flow/src/parser/test/flow") - ); - const result = new Map(); - tests.forEach((section, sectionName) => { - const cases = Array.from(section.values()); - result.set(sectionName, { tests: cases }); - }); - return result; -} - const options = { plugins: ["jsx", "flow", "asyncGenerators", "objectRestSpread"], sourceType: "module", @@ -92,11 +82,11 @@ const flowOptionsMapping = { let failedTests = 0; let successTests = 0; -const tests = get_hardcoded_tests(); +const tests = get_tests(TESTS_FOLDER); tests.forEach((section, sectionName) => { console.log(""); console.log(`### ${sectionName} ###`); - section.tests.forEach(test => { + section.forEach(test => { const shouldSuccess = test.expected_ast && (!Array.isArray(test.expected_ast.errors) || From 656815a53a355768c93b93b325a6e96af7368007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sat, 19 Aug 2017 22:02:27 +0200 Subject: [PATCH 3/5] Add whitelist to flow tests --- scripts/flow_tests_whitelist.txt | 56 ++++++++++ scripts/run_flow_tests.js | 171 +++++++++++++++++++++---------- 2 files changed, 174 insertions(+), 53 deletions(-) create mode 100644 scripts/flow_tests_whitelist.txt diff --git a/scripts/flow_tests_whitelist.txt b/scripts/flow_tests_whitelist.txt new file mode 100644 index 0000000000..aa0d9bafd4 --- /dev/null +++ b/scripts/flow_tests_whitelist.txt @@ -0,0 +1,56 @@ +# This file lists tests that are known to produce incorrect results when parsed +# with Babylon: +# +# - Tests that are expected to parse successfully but for which Babylon reports +# a syntax error +# - Tests that contain invalid syntax but for which Babylon reports no syntax +# error +# +# Entries should be removed incrementally as Babylon is improved. + +ES6/binding-pattern/object-pattern/await-prop-in-async-function.js +ES6/computed_properties/migrated_0000.js +JSX_invalid/migrated_0000.js +arrow_function/param-dflt-yield-expr.js +arrow_function_invalid/migrated_0002.js +async_await/async_generic_method.js +async_await/migrated_0020.js +async_await/migrated_0024.js +async_await/migrated_0027.js +async_generators/migrated_0007.js +class_properties/migrated_0000.js +class_properties/migrated_0005.js +class_properties/migrated_0011.js +class_properties/migrated_0016.js +class_properties/migrated_0021.js +class_properties/migrated_0026.js +decorators/migrated_0003.js +decorators/migrated_0007.js +dynamic_import/migrated_0000.js +dynamic_import/migrated_0001.js +dynamic_import/migrated_0002.js +dynamic_import/migrated_0003.js +dynamic_import/migrated_0004.js +invalid_syntax/migrated_0000.js +invalid_syntax/migrated_0001.js +invalid_syntax/migrated_0002.js +invalid_syntax/migrated_0003.js +private_class_properties/valid.js +types/annotations/migrated_0001.js +types/annotations_in_comments_invalid/migrated_0000.js +types/annotations_in_comments_invalid/migrated_0001.js +types/annotations_in_comments_invalid/migrated_0002.js +types/annotations_in_comments_invalid/migrated_0003.js +types/annotations_in_comments_invalid/migrated_0004.js +types/declare_export_invalid/migrated_0013.js +types/declare_statements_invalid/migrated_0001.js +types/number_literal_invalid/migrated_0000.js +types/parameter_defaults/migrated_0023.js +types/parameter_defaults/migrated_0026.js +types/parameter_defaults/migrated_0028.js +types/parameter_defaults/migrated_0029.js +types/parameter_defaults/migrated_0030.js +types/parameter_defaults/migrated_0031.js +types/parameter_defaults/migrated_0032.js +types/string_literal_invalid/migrated_0000.js +types/typecasts_invalid/migrated_0001.js \ No newline at end of file diff --git a/scripts/run_flow_tests.js b/scripts/run_flow_tests.js index f74df55c62..a8ba4f2def 100644 --- a/scripts/run_flow_tests.js +++ b/scripts/run_flow_tests.js @@ -6,6 +6,7 @@ const chalk = require("chalk"); const parse = require("..").parse; const TESTS_FOLDER = path.join(__dirname, "../build/flow/src/parser/test/flow"); +const WHITELIST_PATH = path.join(__dirname, "./flow_tests_whitelist.txt"); function map_get_default(map, key, defaultConstructor) { if (map.has(key)) { @@ -16,6 +17,14 @@ function map_get_default(map, key, defaultConstructor) { return value; } +function get_whitelist(filename) { + return fs + .readFileSync(filename, "utf8") + .split("\n") + .map(line => line.replace(/#.*$/, "").trim()) + .filter(Boolean); +} + function list_files(root, dir) { const files = fs.readdirSync(dir ? path.join(root, dir) : root); let result = []; @@ -48,15 +57,13 @@ function get_tests(root_dir) { const cases = map_get_default(tests, test_name, Map); const case_ = map_get_default(cases, case_name, Object); - const content = fs.readFileSync(path.join(root_dir, file), { - encoding: "utf8", - }); + const content = fs.readFileSync(path.join(root_dir, file), "utf8"); const ext = case_parts[case_parts.length - 1]; const kind = case_parts.length > 2 ? case_parts[case_parts.length - 2] : null; if (ext === "js") { - case_.name = case_name; + case_.file = file; case_.content = content; } else if (ext === "json" && kind === "tree") { case_.expected_ast = JSON.parse(content); @@ -80,17 +87,31 @@ const flowOptionsMapping = { esproposal_decorators: "decorators", }; -let failedTests = 0; -let successTests = 0; +const summary = { + passed: true, + allowed: { + success: [], + failure: [], + }, + disallowed: { + success: [], + failure: [], + }, + unrecognized: [] +}; + const tests = get_tests(TESTS_FOLDER); -tests.forEach((section, sectionName) => { - console.log(""); - console.log(`### ${sectionName} ###`); +const whitelist = get_whitelist(WHITELIST_PATH); + +const unrecognized = new Set(whitelist); + +tests.forEach(section => { section.forEach(test => { const shouldSuccess = test.expected_ast && (!Array.isArray(test.expected_ast.errors) || test.expected_ast.errors.length === 0); + const inWhitelist = whitelist.indexOf(test.file) > -1; const babylonOptions = Object.assign({}, options); babylonOptions.plugins = babylonOptions.plugins.slice(); @@ -122,53 +143,97 @@ tests.forEach((section, sectionName) => { } catch (e) {} } - if ((!failed && shouldSuccess) || (failed && !shouldSuccess)) { - successTests++; - console.log(chalk.green(`✔ ${test.name}`)); - } else { - failedTests++; - printErrorMessage( - test.name, - exception, - shouldSuccess, - test.expected_ast, - babylonOptions - ); - } + const isSuccess = shouldSuccess !== failed; + const isAllowed = isSuccess !== inWhitelist; + + summary[isAllowed ? "allowed" : "disallowed"][ + isSuccess ? "success" : "failure" + ].push({ test, exception, shouldSuccess, babylonOptions }); + summary.passed &= isAllowed; + + unrecognized.delete(test.file); + + process.stdout.write(chalk.gray(".")); }); }); -function printErrorMessage( - code, - exception, - shouldSuccess, - expectedAst, - babylonOptions -) { - console.log(chalk.red(`✘ ${code}`)); - console.log( - chalk.yellow( - ` Should ${shouldSuccess - ? "parse successfully" - : `fail parsing`}, but did not` - ) - ); - if (exception) - console.log(chalk.yellow(` Failed with: \`${exception.message}\``)); - if (Array.isArray(expectedAst.errors) && expectedAst.errors.length > 0) { - console.log(chalk.yellow(" Expected error message similar to:")); - expectedAst.errors.forEach(error => { - console.log(` • ${error.message}`); - }); - } - - console.log( - chalk.yellow(` Active plugins: ${JSON.stringify(babylonOptions.plugins)}`) - ); -} +summary.unrecognized = Array.from(unrecognized); +summary.passed &= summary.unrecognized.length === 0; +// This is needed because, after the dots written using +// `process.stdout.write(".")` there is no final newline console.log(); -console.log(chalk.green(`✔ ${successTests} tests passed`)); -console.log(chalk.red(`✘ ${failedTests} tests failed`)); -process.exit(failedTests > 0 ? 1 : 0); +console.log("\n-- FAILED TESTS --"); +summary.disallowed.failure.forEach( + ({ test, shouldSuccess, exception, babylonOptions }) => { + console.log(chalk.red(`✘ ${test.file}`)); + if (shouldSuccess) { + console.log(chalk.yellow(" Should parse successfully, but did not")); + console.log(chalk.yellow(` Failed with: \`${exception.message}\``)); + } else { + console.log(chalk.yellow(" Should fail parsing, but did not")); + } + console.log( + chalk.yellow( + ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` + ) + ); + } +); +summary.disallowed.success.forEach( + ({ test, shouldSuccess, babylonOptions }) => { + console.log(chalk.red(`✘ ${test.file}`)); + if (shouldSuccess) { + console.log( + chalk.yellow( + " Correctly parsed successfully, but" + + " was disallowed by the whitelist" + ) + ); + } else { + console.log( + chalk.yellow( + " Correctly failed parsing, but" + + " was disallowed by the whitelist" + ) + ); + } + console.log( + chalk.yellow( + ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` + ) + ); + } +); + +console.log("-- SUMMARY --"); +console.log( + chalk.green("✔ " + summary.allowed.success.length + " tests passed") +); +console.log( + chalk.green( + "✔ " + + summary.allowed.failure.length + + " tests failed but were allowed in the whitelist" + ) +); +console.log( + chalk.red("✘ " + summary.disallowed.failure.length + " tests failed") +); +console.log( + chalk.red( + "✘ " + + summary.disallowed.success.length + + " tests passed but were disallowed in the whitelist" + ) +); +console.log( + chalk.red( + "✘ " + + summary.unrecognized.length + + " tests specified in the whitelist were not found" + ) +); + +process.exit(summary.passed ? 0 : 1); From 748b6fc477cc1568de66efc835ddb86bf7257ae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sat, 19 Aug 2017 22:18:10 +0200 Subject: [PATCH 4/5] Add option to update the whitelist --- Makefile | 3 ++ scripts/run_flow_tests.js | 109 +++++++++++++++++++++++++------------- 2 files changed, 74 insertions(+), 38 deletions(-) diff --git a/Makefile b/Makefile index 3af1841eb6..094ef28876 100644 --- a/Makefile +++ b/Makefile @@ -32,6 +32,9 @@ bootstrap-flow: clean test-flow: node scripts/run_flow_tests.js +test-flow-update-whitelist: + node scripts/run_flow_tests.js --update-whitelist + bootstrap-test262: clean mkdir ./build git clone https://github.com/tc39/test262.git ./build/test262 diff --git a/scripts/run_flow_tests.js b/scripts/run_flow_tests.js index a8ba4f2def..e08ecc3556 100644 --- a/scripts/run_flow_tests.js +++ b/scripts/run_flow_tests.js @@ -8,6 +8,8 @@ const parse = require("..").parse; const TESTS_FOLDER = path.join(__dirname, "../build/flow/src/parser/test/flow"); const WHITELIST_PATH = path.join(__dirname, "./flow_tests_whitelist.txt"); +const shouldUpdateWhitelist = process.argv.indexOf("--update-whitelist") > 0; + function map_get_default(map, key, defaultConstructor) { if (map.has(key)) { return map.get(key); @@ -74,6 +76,25 @@ function get_tests(root_dir) { return tests; } +function update_whitelist(summary) { + const contains = (tests, file) => + tests.some(({ test }) => test.file === file); + + const result = fs + .readFileSync(WHITELIST_PATH, "utf8") + .split("\n") + .filter(line => { + const file = line.replace(/#.*$/, "").trim(); + return ( + !contains(summary.disallowed.success, file) && + !contains(summary.disallowed.failure, file) && + summary.unrecognized.indexOf(file) === -1 + ); + }) + .join("\n"); + fs.writeFileSync(WHITELIST_PATH, result); +} + const options = { plugins: ["jsx", "flow", "asyncGenerators", "objectRestSpread"], sourceType: "module", @@ -164,48 +185,50 @@ summary.passed &= summary.unrecognized.length === 0; // `process.stdout.write(".")` there is no final newline console.log(); -console.log("\n-- FAILED TESTS --"); -summary.disallowed.failure.forEach( - ({ test, shouldSuccess, exception, babylonOptions }) => { - console.log(chalk.red(`✘ ${test.file}`)); - if (shouldSuccess) { - console.log(chalk.yellow(" Should parse successfully, but did not")); - console.log(chalk.yellow(` Failed with: \`${exception.message}\``)); - } else { - console.log(chalk.yellow(" Should fail parsing, but did not")); - } - console.log( - chalk.yellow( - ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` - ) - ); - } -); -summary.disallowed.success.forEach( - ({ test, shouldSuccess, babylonOptions }) => { - console.log(chalk.red(`✘ ${test.file}`)); - if (shouldSuccess) { +if (summary.disallowed.failure.length || summary.disallowed.success.length) { + console.log("\n-- FAILED TESTS --"); + summary.disallowed.failure.forEach( + ({ test, shouldSuccess, exception, babylonOptions }) => { + console.log(chalk.red(`✘ ${test.file}`)); + if (shouldSuccess) { + console.log(chalk.yellow(" Should parse successfully, but did not")); + console.log(chalk.yellow(` Failed with: \`${exception.message}\``)); + } else { + console.log(chalk.yellow(" Should fail parsing, but did not")); + } console.log( chalk.yellow( - " Correctly parsed successfully, but" + - " was disallowed by the whitelist" - ) - ); - } else { - console.log( - chalk.yellow( - " Correctly failed parsing, but" + - " was disallowed by the whitelist" + ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` ) ); } - console.log( - chalk.yellow( - ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` - ) - ); - } -); + ); + summary.disallowed.success.forEach( + ({ test, shouldSuccess, babylonOptions }) => { + console.log(chalk.red(`✘ ${test.file}`)); + if (shouldSuccess) { + console.log( + chalk.yellow( + " Correctly parsed successfully, but" + + " was disallowed by the whitelist" + ) + ); + } else { + console.log( + chalk.yellow( + " Correctly failed parsing, but" + + " was disallowed by the whitelist" + ) + ); + } + console.log( + chalk.yellow( + ` Active plugins: ${JSON.stringify(babylonOptions.plugins)}` + ) + ); + } + ); +} console.log("-- SUMMARY --"); console.log( @@ -236,4 +259,14 @@ console.log( ) ); -process.exit(summary.passed ? 0 : 1); +// Some padding to separate the output from the message `make` +// adds at the end of failing scripts +console.log(); + + +if (shouldUpdateWhitelist) { + update_whitelist(summary); + console.log("\nWhitelist updated"); +} else { + process.exit(summary.passed ? 0 : 1); +} From 79d6bad5c27ac7fb8f0c9705ac5dcd3bb991f311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Sat, 19 Aug 2017 23:13:47 +0200 Subject: [PATCH 5/5] =?UTF-8?q?Disallow=20failures=20in=20the=20flow-test?= =?UTF-8?q?=20JOB=20=F0=9F=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index b9b4ffb1f0..8e80a0b41c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,6 @@ matrix: env: JOB=flow-test - node_js: node env: JOB=test262-test - allow_failures: - node_js: "lts/*" env: JOB=flow-test