Tests fail on v0.19.0, stack overflow in test_large_example

[alerque]

Trying to update Arch Linux packaging to v0.19.0, I've run into the following error in our check phase:

$ cargo test --locked --offline
...
thread 'test_large_example' has overflowed its stack
fatal runtime error: stack overflow
...

[JohnnyMorganz]

Aargh sorry about that; forgot that downstream packagers will also be affected.

Can you add --release to your test check? Should get rid of the stack overflow.

Underlying issue is in an upstream crate: Kampfkarren/full-moon#140

[alerque]

We can hack around this that way, but your solution introduces other problems. Running cargo test --release overwrites the binary generated by cargo build --release, and in our case ends up compiling with different features.

[JohnnyMorganz]

Hm fair point, it's a shame this is a problem in the first place.

Although I would argue that the tests should be running with the same feature flags that the final release ends up being built as, otherwise the testing is technically incomplete. (Actually our testing is messy wrt feature flags, so testing with just flags is not completely right - a thorough test would be testing with both default flags and then relevant flags separately)

[alerque]

Cargo has made some really unfortunate decisions along the way. They have improved, but there is still room. It used to be that cargo test --release would actually not use the same profile as cargo build --release, it actually mixed in profiling stuff from the bench profile. This combined with cargo test overwriting the binaries from cargo build led many distro packages to go out with profiling stuff baked in their binaries.

The current "fix" is to manually specify separate output directory trees (target) for build vs. testing.

But I don't understand why your cargo test suite is setup in such a way that a runtime stack overflow is an expected result. Maybe gate the test behind a config flag if optimization is required to pass the test.

[JohnnyMorganz]

But I don't understand why your cargo test suite is setup in such a way that a runtime stack overflow is an expected result. Maybe gate the test behind a config flag if optimization is required to pass the test.

Very valid question, it didn't use to stack overflow, but a dependency bump in the last update caused this. You are right though, it shouldn't be part of the test suite if it's failing (for unrelated reasons!). I'll gate it tomorrow

[JohnnyMorganz]

Should be resolved by 711e4a8

Would it be useful for you if we cut a new release, or is it fine for now? Happy to do so

[alerque]

We've already hacked Arch Linux to work, but there are other distros out there that either haven't updated or maybe have tried and ran into trouble. More people are bound to run into this, and the most likely people to be troubled by it are distro packagers. Those are probably the folks you want to keep on your side with a quick patch release sa that everything is expected to work instead of known-bad.

[JohnnyMorganz]

Released https://github.com/JohnnyMorganz/StyLua/releases/tag/v0.19.1