ciscoheat / buddy

Your friendly BDD testing library for Haxe!
MIT License
96 stars 24 forks source link

dce remove StringMap.keys() method and fail utest Assert #24

Closed profelis closed 10 years ago

profelis commented 10 years ago

I use utest library with buddy. utest has this lines

https://github.com/fponticelli/utest/blob/d8ab198807434549f172dc18b157ec49327eb0cf/src/utest/Assert.hx#L289-L290

where keys() call without casting to StringMap or IntMap. haxe compiler (dce) remove keys() method as unused.

ciscoheat commented 10 years ago

Hi, is there anything I can do about this? Anything that needs to be done in Buddy?

profelis commented 10 years ago

Yes! You can fix it in buddy. Exists some ways to fix this problem 1) turn off dce in http://haxe.org/manual/haxelib-extraParams.html 2) only add @:keep meta for this classes http://haxe.org/manual/cr-dce.html in extraParams.hxml 3) somethere in Buddy call StringMap.keys() and IntMap.keys(), and dce doesn't remove this methods

I recommend second fix

ciscoheat commented 10 years ago

Now I understand the problem, thanks. Any problem with nr 3? Since I don't want to limit the compilation options for anyone. I can detect if utest is being used and call keys() for the classes if so.

profelis commented 10 years ago

I don't see problems with 3th. I opened issue in utest library https://github.com/fponticelli/utest/issues/9 , maybe they fix it in main library

ciscoheat commented 10 years ago

Ok, I've uploaded version 0.14.1 now (haxelib) using fix number 3. Can you test and see if it works?

profelis commented 10 years ago

No, doesn't work. Dce doesn't see keys() call and remove this method, but if I call keys() manual, I get new error

ReferenceError: Error #1069: Property get not found on haxe.ds.IntMap and there is no default value.)
    @ /Users/deep/dev/haxe/haxe/lib/utest/1,3,0/src/utest/Assert.hx:298
    @ /Users/deep/dev/haxe/haxe/lib/utest/1,3,0/src/utest/Assert.hx:499
    @ /Users/deep/dev/haxe/hml/samples/test/src/tests/UITests.hx:53
    @ /Users/deep/dev/haxe/haxe/lib/buddy/0,14,1/buddy/internal/SuiteRunner.hx:177

Now I start to think, that second fix is more right

profelis commented 10 years ago

this code

var s = new haxe.ds.StringMap();
        var i = new haxe.ds.IntMap();
        // store links for methods, because dce remove inline methods
        var q = s.keys;
        var q = i.keys;
        var q = s.get;
        var q = i.get;

temporar resolve problem

profelis commented 10 years ago

bug case

it ("utest should compare maps correct", {
    utest.Assert.same([1 => 2], [1 => 2]);
    utest.Assert.same(["a" => 1], ["a" => 1]);
});

but this test you must run without other tests

ciscoheat commented 10 years ago

All right, version 0.14.2 uploaded, it should do it then. Thanks for the code examples. :)