Compatibility with 0.4 syntax, badges, test coverage

[garborg]

The test coverage badge may not get stats filled in until after the merge -- the % doesn't mean as much to me, but I think it would be great to see what paths aren't tested so we can add relevant tests before changing anything tricky.

[garborg]

Curious to see what we get.. I'll merge it for now.

[garborg]

I guess coverage stats don't start until the next pull. For shame.

[tedsteiner]

This coverage stuff is pretty cool! I didn't know this type of thing was available.

I'll take a look at it in more detail soon, but it looks like we actually have better coverage than the percentage applies, just because of stuff like in highways, my sample map we use for testing doesn't have some map features, like roundabouts, so those lines aren't tested. And for plot.jl, there are just so many options. But we could test them all, I suppose. It's not like it's difficult to do. And for some of the other files, it looks like we'd have to trigger the exceptions to get full coverage? Interesting. But what's cool is that it looks like everything important is already covered, so that's pretty comforting. Thanks for doing all this, it looks like you spent a lot of time getting it working!

[garborg]

Yeah, I was pretty impressed by the coverage, too.

On the other hand, relying too much on the numbers can be misleading -- I just saw that that segmented networks aren't being tested at all despite being fully covered (copy-paste error in test, fixing now), and calling a complicated function in a test file is a couple steps removed from making sure that the subsequent tests are sufficient.

More coverage is great (in the far term, I'd be for having a couple small test maps to make sure we cover all the features we handle at that point). But continuing to be cognizant of what the tests actually catch seems more important (not telling you anything new -- just putting it in the books to assert that we're conscious of implications 'gamified' testing).

[garborg]

Saw something else to watch for -- Julia's code coverage tool isn't great at marking which lines are 'important' -- see types.jl lines 102-139, where a couple functions, seemingly arbitrarily chosen, are green, but many more aren't marked as important, so we don't know whether they were called.

It's the first iteration of code coverage in Julia, and the issue is known -- just a matter of developers working through higher priorities first.

[tedsteiner]

So green means important and red means untested, correct? I agree that it seems strange that those lines are the chosen ones. But it might be that those are the actual lines of code rather than type definitions. It makes sense that the Style constructor would be used a lot, for example. Maybe if we had an explicit constructor for every type we would get confirmation that every type is being tested, but I don't think it's necessary, and probably is just code clutter.

Thanks for the heads up on this, this testing stuff is pretty new to me but very exciting.

[garborg]

I'm not sure this fits the actual mechanics, but in terms of desired outcome, I believe the tool means to mark all functions as 'important' / worthy of tracking (i.e. not a line with nothing but 'end', not a type without an explicit constructor, etc.), and then track counts for each important line.

If the tool was working as desired, all lines with anything to do would be green if its count was > 0 or red otherwise. So it's a failure of the tool to track getX(::LLA) =... but not getX(::ENU) = .... The lines that are highlighted are colored accurately, though, so still super useful -- just hopefully the tool eventually starts telling us whether those unhighlighted functions are red or green.

I definitely agree about avoiding code clutter. Code coverage has kind of gotten me to avoid the popular but controversial idiom of short circuiting conditionals (isempty(x) || do_something(x) for if !isempty(x); do_something(x); end, but only because it was unclear which was better / more idiomatic style in the first place.