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 inline #3915

Merged
merged 1 commit into from May 21, 2020
Merged

fix corner case in inline #3915

merged 1 commit into from May 21, 2020

Conversation

alexlamsl
Copy link
Collaborator

fixes #3911

@kzc
Copy link
Contributor

kzc commented May 21, 2020

I couldn't repro the test failure without this PR until I remembered to add the --validate flag. It's a clever way to enforce AST sanity checks without slowing down general use. Kudos. I wonder how many single element sequences and statements within expressions snuck through over the years without causing immediate problems. I guess ufuzz will show us soon enough. It still surprises me when a 5+ year old bug surfaces.

@alexlamsl
Copy link
Collaborator Author

As NASA would say, "this is why we test." 🚀

@alexlamsl
Copy link
Collaborator Author

... having said that, this is the second time I encounter this − https://github.com/mishoo/UglifyJS/runs/696063068

The first case happened earlier this week.

@kzc
Copy link
Contributor

kzc commented May 21, 2020

Users have no idea how extensive the testing is. I'm not aware of another open source JS project with the same level of fuzz testing.

I'm too lazy to look at the code... is validate run in ufuzz for each and every minify and original beautify or just in the event of a minify result difference? I suspect the former, but thought I'd ask.

@kzc
Copy link
Contributor

kzc commented May 21, 2020

... having said that, this is the second time I encounter this −

not sure what I'm looking at - it's a github actions bug of some kind?

@alexlamsl
Copy link
Collaborator Author

not sure what I'm looking at - it's a github actions bug of some kind?

I'd think so, as it reports ufuzz for an hour which seems nominal − only the logs are missing.

"View raw logs" shows pretty much nothing either.

@alexlamsl
Copy link
Collaborator Author

is validate run in ufuzz for each and every minify and original beautify or just in the event of a minify result difference?

We validate everywhere except when beautifying code in try_beautify() − however beautify: true is included in test/ufuzz/options.json so that should already be covered.

@alexlamsl alexlamsl merged commit aeb9ea5 into mishoo:master May 21, 2020
@alexlamsl alexlamsl deleted the issue-3911 branch May 21, 2020 14:05
@alexlamsl
Copy link
Collaborator Author

Another one bites the dust: https://github.com/mishoo/UglifyJS/runs/703663819

@kzc
Copy link
Contributor

kzc commented May 24, 2020

While annoying, the github bugs aren't as concerning as the recent unreproducible NodeJS bugs. But the latter problem may be remedied by upgrading the version of NodeJS used in CI.

@alexlamsl
Copy link
Collaborator Author

I downupgraded from latest version of Node.js exactly because of V8 bugs... 😏

@kzc
Copy link
Contributor

kzc commented May 24, 2020

Oh well. Even with Google's near infinite fuzzing resources some bugs are bound to slip through.

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.

ufuzz failure
2 participants