Coverage and line counting issues.

[olegskl]

I'm continuing working on the HTML reporter and starting to get visual representation of our code coverage. There are some issues, mostly due to the fact that the lines function in adana-analyze reports all lines between loc.start and loc.end as covered with coverage count of the parent node. This not only results in empty lines having coverage count, but also in the fact that empty lines have higher count than their neighboring lines with statements and the such (compare lines 18 and 19).

So, I changed the line counting loop to only include loc.start and use Math.max on counts. This solves two issues:

1. The lines reported as covered are actually covered.
2. Lines with function assignments actually show the count of function declaration, not the count of function executions. Same goes for a single line ternary expressions. If the falsy branch of a ternary is not covered, that doesn't mean that the ternary is not executed at all.

Below is the screenshot of the result. It's only a little bit better.

1. I don't know what to do with functions. On one hand I think it's good that we're tracking the execution count for the "function" tag, but the visual representation is weird (see line 2).
2. Method calls don't seem to be covered.
3. Something wonky is going on with conditionals (see lines 11 and 13).

[izaakschroeder]

Thanks for highlighting all this! I have struggled with the lines algorithm myself still in determining what actually makes the most sense for some of these cases. I'll see if I can review this later tonight and maybe we can come up with some rules.

You may find it handy to include a list of locations hit in your metrics for lines (e.g. line 4 hit locations 5 and 7) – that will give you insight into some of the "magic". My guess for the magic is that adana is injecting and counting its phantom "else" branch there and both are being counted for that line. I don't know that for sure, but that's my guess.

I'm not sure what to do about certain expressions – I feel like an expression statement (in general) should cover all lines in that statement (e.g. in yours the line 19 statement should cascade through to line 32). I don't know if I want to add coverage for every single operator? Maybe? It could be done certainly (and at present there is no markers added for operators of any kind). It would enhance the accuracy of statements like:

const x = 
  foo() +
  bar() + // if this line throws an error then baz won't be covered 
  baz();

Also you should totally move your html formatter repo over to the org if you'd like! I'll delete the shell one I have and yours can replace it. :smile:

[olegskl]

I had never written a babel plugin before, so in order to learn I started a rewrite of adana transformer from scratch and came up with a version that seems to work well for the app we're working on here.

Below is a screenshot of the code coverage with my version of adana. It's quite similar to the report that we get from Istanbul, but the big plus is that the branch count is better (Istanbul reports 20 branches because it is based on the transpiled results).

I'm also adding an example that has classes, super and destructuring:

I'll clean up the code and upload it so that you can take a look.

[olegskl]

Here it is: babel-plugin-transform-adana-alt. I really should have done it as a fork, but at the time I thought I was just going to play around... I will prettify it and make a proper MR if we decide to use it.

Notable changes:

• I moved visitors and instrumenters to separate files and import types directly from babel-core. Is it a bad idea? I've seen some babel plugins do it. Must be safe, I guess.
• I dropped the file hashes. Are they necessary? What's the use-case?
• I dropped the tags and rules. Are they necessary? I don't really understand their purpose.
• I don't have function names or conditional groupings (yet). I guess I really need to start working on the html reporter and see what kind of data is needed to display relevant coverage information.

Also there's Tape for unit tests, but I can switch to Mocha if you think it's better. And for linting I used eslint-config-meetic because we're actively using it in our projects (and frankly I'm not really a fan of metalab config, because it changes too much from one version to another, has a dependency on react plugin and v2 wants me to use function expressions for some reason).

And I really want to change the name "prelude.js" to something else... have no ideas yet.

[izaakschroeder]

Phew lot to go over :smile:

• I like the idea of having separate files for each bit (easier to test not being wrapped in a closure too). I am not sure what the proviso is about importing straight from babel-core. If it's ok then I say :+1:.
• File hashes are to let you merge coverage objects. Since you can only merge coverage objects if their sources are identical, hashes made the most sense. You might want to merge coverage objects if you're using hot-reloading in your tests – i.e. you want to keep all existing coverage data except for the things that have changed when you've done the reload.
• Tags are one of the defining features of adana (at least in my mind). The instrumenter won't work without them. By "drop tags" I assume you mean "drop user-defined tagged" since function and statement are also tags of a sort. Tags will definitely be necessary when people start asking how they can ignore lines or statements. There are other use cases for tags like ensuring your FireFox-specific tests actually get hit by FireFox.
• I don't really have that strong an opinion on test frameworks. Everything else I've done has used mocha. The only real hard requirement here is that you have to show that adana can generate coverage about itself.
• I (primarily) maintain the metalab one and it's the one we use on our projects. The peerDependency on the plugins is annoying but it's a problem with eslint. You shouldn't have to install those plugins but due to the way eslint works that's how it's gotta be for now. It did change significantly from v1 to v2 after learning things we liked and things we didn't when using it... but that is the whole point of semver isn't it? :smile: The reasoning behind most of the rule choices is available in the rule files or the PRs in that repo if you want to understand why some choice was made.

[izaakschroeder]

My recommendation is to (in priority order):

• Port over the most important functionality changes first – i.e. any logic changes that have happened. They will be easier to review in isolation. Things like adjustments to the instrumenter or new features.
• Add in any additional documentation you've created – e.g. I see in the test fixtures folder you've added a bunch of comments about things. Valuable asset to other people looking at the code.
• Then after those are ok, handle the structural changes – e.g. moving things into their own files. It will be a noisy PR but at least it will be clear functionality is not changing.
• Lastly any stylistic changes – e.g. moving from mocha to tape or changing lint configuration or whatever. These are essentially bike-shed issues and don't really add or remove value. And ultimately it's probably the least valuable use of your time. :smile:

Additionally at some point (if you want):

• Move over whatever other libraries you've made that work with adana. The HTML reporter being a prime example. I have no requirement that your repos living under this org have to follow some standard regarding linting or structure or whatever – they are yours after all.

Thanks again for all the hard work and interest in this!

[izaakschroeder]

Also as a total aside regarding tags: it might be interesting to write a function in adana-analyze that collects all the tags for a given line. In your HTML formatter you could then generate color codes for each tag. e.g. blue for statement, purple for branch and so on. For each line instead of a huge green bar you display colored stripes. You could desaturate an individual tag color on a line if the count is 0 for that tag. This would be a good segway into showing how user-defined tags can be used (basically a mechanism to group coverage information together) – you can visualize things like "show me all coverage dealing with compression" (which maybe in this case means you hide every other color or dim all lines without the "compression" tag). There a lots of possibilities! :tada:

[olegskl]

Thanks for explanations! I the user-defined tag idea is great. (Side note regarding tags: I've made a PR to adana-analyze because it threw an error when I attempted to instrument a class constructor).

There are some ideas for a more interesting highlighting of the code and functionality in the html reporter (like ranges and drill-down on click or mouseover), but there's just not enough time to do it all. I'll get to it eventually.

Let's stick to Mocha for the time being.