basho / cuttlefish

never lose your childlike sense of wonder baby cuttlefish, promise me?
Apache License 2.0
205 stars 124 forks source link

Fix/erlang18 #197

Closed trevorriles closed 9 years ago

trevorriles commented 9 years ago

This fixes #196 (RIAK-2055).

Would be nice to get a new neotoma release so we don't have to track master.

trevorriles commented 9 years ago

This duplicates #175 (RIAK-1966) with an additional dependency fix.

Can anyone test on Erlang 17?

macintux commented 9 years ago

I'm getting the utf8 test failure under both 17 and 18, even with the updated neotoma tag.

macintux commented 9 years ago

Ah, neither make clean nor make distclean remove conf_parse.erl.

macintux commented 9 years ago

Passes under 17. Looking into whether we should grab a newer version of lager.

macintux commented 9 years ago

@trevorriles If you could also make the lager tag 2.2.0, I'll approve this

trevorriles commented 9 years ago

@macintux the lager dep has been updated.

macintux commented 9 years ago

Looks like that perhaps the utf8 test not fails under R16, digging.

macintux commented 9 years ago

Yep. Unicode confusion. If conf_parse.erl is tweaked to have a utf8 file encoding, the test will pass; similarly, if the character is specified explicitly as its utf8 encoding, the test passes:

-    Conf = conf_parse:parse("setting = thingŒ\n"),
+    Conf = conf_parse:parse("setting = thing" ++ [338] ++ "\n"),
macintux commented 9 years ago

Once https://github.com/basho/cuttlefish/pull/199 is approved and merged, this PR's integration branch tests should also pass, allowing a merge.

macintux commented 9 years ago

Well, not quite as easy as I'd hoped. Could you rebase your branch?

trevorriles commented 9 years ago

sure thing, I'll rebase

trevorriles commented 9 years ago

alright, I rebased my commits on top of develop.

macintux commented 9 years ago

Going to do more digging. I'm seeing some errors under 18 that concern me; the tests pass, but lager isn't happy.

Saw similar errors under 17.

https://gist.github.com/macintux/fb9f8710cb3f38085ebe

These didn't show up every run.

trevorriles commented 9 years ago

Are there specific errors that you are seeing. A lot of those look like the tests intentionally call them to make sure we behave on improper configuration.

Anything I can do to help?

macintux commented 9 years ago

I can't reliably reproduce these.

There are indeed lots of error messages deliberately introduced, but these are showing up as lager crash reports, of particularly interest since we maintain that, too.

Anyway, will give this the thumbs up and worry about the lager reports later.

macintux commented 9 years ago

@borshop merge