A few tests missing from test suite

[mrclay]

https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue144.js
https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/condcomm.js
https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue132.js
https://github.com/mrclay/minify/blob/master/min_unit_tests/_test_files/js/issue74.js

Supporting things like a - ++b required quite a patch in JSMin.

I'm still open to using this in Minify if it can handle all the same content and (ideall) faster.

[tedivm]

I really, really want to build out the test suite for JShrink, and it's in my personal development queue. Right now most of my focus has been on trying to get Stash up to snuff for potential inclusion in Symfony, as well as the FIG standard for caching, which is why I haven't spent as much time on this as I'd like.

I'm avoiding looking at the JSMin source due to it's crazy license- if you think it would help I'd love to hear about the approach you took for this.

[tedivm]

@mrclay -

I've added issue114,js and issue132.js to my test suite, and JShrink is passing. This will be coming out in my next release. This covers your "a - ++b" case, which as you mention did require quite a patch.

Looking for issue74.js gave me a 404 error, so I can't add that. If you could point me in the right direction I'd appreciate it.

With the "condcomm.js" I'm a little confused- it looks like your version strips out vital information. In your input test file on line 9 there is this-

isWin = /*@cc_on!*/!1;

That isn't accounted for anywhere in your output file- it's literally ripped out. You probably didn't notice if you were testing in Internet Explorer because this evaluates to "isWin=!1", which is the same as "isWin=false" and matches the immediate javascript above it.

Additionally the jsmin version doesn't seem to do much minifying around the conditional comments, which JShrink does manage to do and it does so without breaking the conditional comments.

In other words, I think my conditional commenting code is a complete solution, even though we get different output.

[tedivm]

I think you generated that test with something other than jsmin that is a bit more active, and that it removed the code itself realizing that it wasn't being used.

I've modified the test accordingly and added it to the test suite. That means that at this moment all three tests pass, and if you want to provide the fourth I'll see what I can do about that.

[tedivm]

Bump: @mrclay, any thoughts?

[mrclay]

I can't remember what I was going to do here.

[tedivm]

Ideally switch Minify over to using JShrink as it's main minifier, or submit more issues that it needs to resolve first.

You had brought these issues up as incompatibility problems that Minify had encountered in the past and built test cases for. I included your test cases directly into my test suite and various people submitted code to make JShrink pass. Things like a - ++b work as expected now.

I also addressed that our conditional commenting code has different output, but that the code in jsmin (modified or not) is buggy and that the test case for it simply enforces the bug. JShrink leaves more of the javascript itself intact while still removing more of the superfluous code, which makes the test case written for jsmin incompatible. The one for jsmin was modified though so the same issue is being tested for in JShrink now.

[mrclay]

Thanks a lot for the recap :)

[mrclay]

The file that showed isWin = /*@cc_on!*/!1; stripped out was actually the output of JSMin+, an alternate implementation without full conditional comment support.

I test this in before.js -> before.min.js (yes, the test suite in Minify is a real mess).

[mrclay]

I've added a few issue to track separately. Thanks for keeping me on task!