spec runner depends on libxml

[straight-shoota]

To run specs, you need to require spec which includes junit_formatter. That formatter requires xml which depends on libxml2 being available. That package is not installed by default - at least not on Debian/Ubuntu or homebrew (the APT package has it as a a suggested dependecy). Didn't check others but I don't expect them to be different.

This means, right after installing Crystal using the provided packages, you're not able to run specs. Instead the compiler will print a very cryptic error message (like posted in #5658):

/usr/bin/ld: cannot find -lxml2
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o '/home/Mead/.cache/crystal/crystal-run-spec.tmp'  -rdynamic  -lxml2 -lpcre -lm -lgc -lpthread /usr/share/crystal/src/ext/libcrystal.a -levent -lrt -ldl -L/usr/lib -L/usr/local/lib`

For beginners, this is not trivial to understand. The error message could obviously be improved (see #5291) but that won't solve the problem that you can't use a major feature without manually installing additional libraries.

This is especially iffy considering the JUnit formatter is a very specific feature and won't be needed for most use cases at all.

A possible solution would be to include libxml2 in the dependencies of the OS packages for Crystal. But I don't think that's the best way because many developers probably won't really use xml anyway so it'd just add an unneccesary dependency.

I think it would be better to make the junit formatter optional. Specs are usually compiled and run using crystal spec, so it should be possible to only require junit_formatter in the code being compiled if it is configured to use that formatter (as CLI argument).

Or remove it entirely from the stdlib into a shard as suggested in #5215.

Related to #5291

[RX14]

The solution in my eyes is just to dump the spec data into JSON, then have an optional external tool which can reprocess that JSON into the junit XML.

[ysbaddaden]

Or manually require "spec/junit" in a spec_helper.cr when the feature is needed?

[straight-shoota]

@ysbaddaden That would also be possible. Maybe this could be the easiest solution.

As I understand from the PR adding this feature, #2455, junit output is generally useful when running on circle-ci. I'm not sure it would be so great to make the code for specs depend on the environment where they are run.
Using a command line arg would make this possible without directly changing any code.

[asterite]

How can a command line arg change that? The only thing that comes to my mind is a -D flag... but I wouldn't do that.

[j8r]

Moving to a shard may be the cleanest solution – why there is specific code to JUnit in the stdlib?!

[straight-shoota]

@asterite If crystal spec is invoked with --junit_output, it could just add spec/junit to the sources.

This obviously won't work if a spec file is run/build with crystal run or crystal build. For that usecase, you'd still have to add it directly to the code.

[asterite]

Another solution is to implement XML in Crystal

[RX14]

Junit is a standard format generated by loads of spec runners and parsed by many tools and ci runners. That's why it's in the stdlib.

I also think that the output formats available should be dictated by the user at the commandline, not by having to require more stuff. Implementing xml in crystal would be ideal, perhaps it would be enough to just template together the xml without a proper builder for this limited case.

[straight-shoota]

You mean essentially reverting #4090?

[asterite]

Please let's not manually generate XML. I see no problem with this being a shard. JUnit test output isn't something built-in in any language I know.

[straight-shoota]

Yeah, but which language has a proper spec library built in?

I don't think manually creating it would be that bad. It's really trivial markup.

I would really prefer that over having to add an extra shard for such a simple feature.

[asterite]

D, Rust, Go and Ruby all have a built-in way to run tests. I guess Nim too.

[straight-shoota]

Nim actually comes with a JUnit formatter that creates XML markup manually.

[asterite]

Cool! Then maybe the easiest solution is to just write that manually (I think it originally was like that and it was working fine).

[luislavena]

Please let's not manually generate XML.

That is good to know, specially since the JUnit output was originally manually generated

But being a bit more serious, I think JUnit support can remain by doing some refactor on spec.cr entry point and option parsing, to allow the require of spec/junit to extend it with --junit_output option and formatter.

Bear with me with the needed changes:

1. In Crystal::Command#spec, detect if --junit_output is used
2. If so, prepend spec/junit require (when building sources)
3. Modify spec.cr entry point so options can be extended (what spec/junit will do)

Will something like that work? I'm missing any other detail?

Cheers.

[straight-shoota]

@luislavena That's what I had initially proposed. But it would actually be better to just implement the JUnit formatter without a dependency on libxml.

[asterite]

The problem with --junit_output is that it won't work with other spec frameworks. These could provide the feature, but I can't see how --junit_output will require something relative to those frameworks.

[luislavena]

@asterite then perhaps will be good to introduce the refactoring (to allow extending spec.cr CLI) and then JUnitFormatter moved to a shard.

Then anyone wanting to use it, simply add it to their spec_helper.cr.

In that way Command#spec doesn't need modification at all.

[luislavena]

@straight-shoota are you already working on this or plan to? I might want to hack on some Crystal this week and this could be a nice thing for me to tackle.

Let me know so we don't overlap our work.

Cheers.

[straight-shoota]

No, go ahead ;)

[bararchy]

@luislavena any news here? it seems we get more people running into this issue

[luislavena]

@bararchy my first attempt was to refactor spec runner in #5763 but didn't move forward due bad reception from the team.

It seems #6004 proposes a direct change to the formatter, which most likely will get merged before mine.

Cheers.

[Sija]

@bararchy my first attempt was to refactor spec runner in #5763 but didn't move forward due bad reception from the team.

To be correct, just from @RX14, not the whole team. Given the positive number of 👍/ ❤️ it received I reckon it should be followed up upon.

[luislavena]

@Sija not criticizing, but definitely will send a RFC about complete changes in Spec CLI to be able to test and hook it more easily.

But will do that once I have time, lately busy with real life 😁

[straight-shoota]

Yeah, I guess it makes sense to discuss some ideas about further improvements for specs first 👍