Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix corner case in reduce_vars #4796

Merged
merged 1 commit into from
Mar 17, 2021
Merged

Conversation

alexlamsl
Copy link
Collaborator

@alexlamsl alexlamsl commented Mar 17, 2021

@kzc another one from harmony − current patch for test/compress.js

--- a/test/compress.js
+++ b/test/compress.js
@@ -14 +14 @@ var batch = 50;
-var dir = path.resolve(path.dirname(module.filename), "compress");
+var dir = path.resolve(path.dirname(module.filename), "../../UglifyJS-harmony/test/compress");
@@ -141,0 +142 @@ function parse_test(file) {
+                    "reminify",
@@ -193,0 +195,4 @@ function reminify(orig_options, input_code, input_formatted, stdout) {
+        if ("keep_fnames" in orig_options) {
+            options.keep_fnames = orig_options.keep_fnames;
+            delete options.rename;
+        }
@@ -255,0 +261,11 @@ function test_case(test) {
+    [ "ecma" ].forEach(function(name) {
+        if (name in output_options) delete output_options[name];
+    });
+    if ("bracketize" in output_options) {
+        output_options.braces = output_options.bracketize;
+        delete output_options.bracketize;
+    }
+    if ("safari10" in output_options) {
+        output_options.webkit = output_options.safari10;
+        delete output_options.safari10;
+    }
@@ -308,0 +325,13 @@ function test_case(test) {
+    [
+        "computed_props",
+        "ecma",
+        "unsafe_arrows",
+        "unsafe_methods",
+        "warnings",
+    ].forEach(function(name) {
+        if (name in test.options) delete test.options[name];
+    });
+    if ("keep_classnames" in test.options) {
+        test.options.keep_fnames = test.options.keep_classnames;
+        delete test.options.keep_classnames;
+    }
@@ -319 +348 @@ function test_case(test) {
-    if (expect != output_code) {
+    if (0 && expect != output_code) {
@@ -359 +388 @@ function test_case(test) {
-    if (test.expect_warnings) {
+    if (0 && test.expect_warnings) {
@@ -390 +419 @@ function test_case(test) {
-        if (test.expect_stdout === true || test.expect_stdout instanceof Error && test.expect_stdout.name === actual.name) {
+        if (1 || test.expect_stdout === true || test.expect_stdout instanceof Error && test.expect_stdout.name === actual.name) {

@kzc
Copy link
Contributor

kzc commented Mar 17, 2021

Is that the last of the issues unearthed by uglify-es? So it only found two problems?

In my compress test runner if process.env.IGNORE_EXPECT is set, it skips the tests without expect_stdout and it ignores expect: and expect_exact: altogether. That way it can run tests from uglify-js, uglify-es and terser.

I did notice that you added class support to keep_fnames. That's good. You get some extra granularity with keep_classnames, but it's not a big deal.

@alexlamsl
Copy link
Collaborator Author

alexlamsl commented Mar 17, 2021

Is that the last of the issues unearthed by uglify-es? So it only found two problems?

Oh #4790 onwards are all results of the scan thus far. And I think there are a few more further down − I'm only up to reduce_vars.js

@alexlamsl
Copy link
Collaborator Author

alexlamsl commented Mar 17, 2021

Oh and I took out tests in export.js and harmony.js that parser would throw (after confirming SyntaxErrors from Node.js)

@alexlamsl alexlamsl merged commit d837a46 into mishoo:master Mar 17, 2021
@alexlamsl alexlamsl deleted the reduce_vars branch March 17, 2021 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants