Multiple engines requiring each other only show their own files in the reports

[mrcasals]

Hi!

First of all, thank you all for your work! Simplecov is an awesome gem <3

Now for the downsides, I'm having some trouble configuring it in our Project. I'm working in Decidim (https://github.com/decidim/decidim), a platform to empower participatory democracy. It's actually a gem made of multiple gems (just like the rails gem does, it installs a bunch of gems). We're using a mono-repo approach to organize the code, each folder is a different gem. I've seen @deivid-rodriguez around, he's worked on the project too in the past 😄

In our case, we have a set of gems that build upon a central one (decidim-core). This means each gem uses code from decidim-core. Each gem has its own spec_helper.rb, although they all look almost the same. We configure SimpleCov through the .simplecov file in our root folder. We run the test suites individually through Github Actions. Our current setup runs each gem test suite independently in different workflows (and thus in different file systems), so only one test suite is run at a time. This is effectively the same (for testing purposes) as running a single test run in your machine. We're not mergint the test suite manually. Instead, this merging is done by Codecov.

Now, whenever I run the tests for one of the gems I'd expect the report to include files from the module and from decidim-core. For example, running the tests from decidim-proposals should include decidim-core files, but it doesn't. I'm playing around SimpleCov configurations in decidim/decidim#5934, but so far the reports only show files form the given module, no extra ones (even though I've set SimpleCov.root correctly). I've also tried the snippet in the README file to load files from engines, but without any luck.

Since we're exporting the coverage reports to Codecov, I'm using the simplecov-cobertura formatter, which is the one recommended in the Codecov docs. I've checked the internals of that gem and it doesn't seem to filter files, so I think the problem comes from a misconfiguration in our part.

If you'd like to check the test suite locally, these are the required steps:

1. Clone the repo and cd into it
2. bundle install
3. bin/rake test_app
4. CI=1 SIMPLECOV=true bin/rspec ./decidim-proposals/spec/system/proposals_spec.rb:50
5. Check the coverage/coverage.xml file

This step I'm using is a system spec that checks a #show page for a resource. This page uses code from decidim-proposals and decidim-core, but if you check the coverage file you'll see how it only shows code from decidim-proposals.

Technical details:

• Ruby 2.5.3p105
• SimpleCov 0.18.5 (latest)
• simplecov-cobertura 1.3.1 (latest)

PD: Feel free to change the issue title if you can come up with a better explanation! Also, ping me if something's not clear enough!

[mrcasals]

After commit b08f14b in the PR in my repo, I see a couple of things, which I don't know if should be considered as issues or not:

1. SimpleCov.track_files doesn't seem to use SimpleCov.root. In our case, we moved SimpleCov.root to an upper folder, but SimpleCov.track_files doesn't take all files into account. Instead, we had to manually change it to include the files in the same folder.
2. Now that we're correctly tracking all the files, our coverage reports still don't show any hit in files from other folders. Should we manually require all Ruby files?

I feel I'm missing something in the definition of "tracked files". For what I see, you want to use "tracked files" in order to ensure you're not missing any files in the report. But this does not ensure hits will be counted.

I'll try manually requiring all Ruby files, see if that improves something.

EDIT: I tried requiring all ruby files in my repo, but that caused loading issues and abandoned this path. I feel I'm missing something, but I can't see it 🙈

[mrcasals]

Possibly related to #572, BTW

[PragTob]

@mrcasals 👋 Hi there and thanks for the report! 👋 💚

Yes, tracked_files doesn't have the best of documentations - one of the things in the way of a 1.0 release 😅

So track_files really just makes sure they are included even if they aren't required. The way the underlying coverage library works is to just add everything that you required to the report. Now, usually you don't want to see that for your gems hence the "root filter" was invented.

To me it sounds like a problem/combination of requiring files after simplecov starts and possibly the root filter. Otherwise files should be shown.

Not super helpful right now but maybe it already helps you figure this out. Depending on things I might also have time to investigate myself :)

Anyhow, thank you!

[mrcasals]

Hi @PragTob, thanks for the warm welcome! 👋

I tried what @NullVoxPopuli did on #572 (comment) and I get 3 calls for a given file. This is what made me point to #572 as a possibly related issue. Unfortunately, though, I'm not sure how to check whether this is really true, and how to fix it 😞

Decidim is complex, and maybe this complexiity is what's getting in our way. For example, we have to use a dummy Rails app to run the tests (at least some of them). This app requires the gems via a :path option in its Gemfile, and maybe this is making the files reload. So, from the top of my head, what I suspect is happening is:

1. When I run the tests from a gem it loads the files inside.
2. It loads the dependencies
3. Then it boots the test app
4. And loads its dependencies, thus reloading the files
5. At some point, files get reloaded again for some reason

That would explain why I'm getting 3 loads per file, but I'm just guessing here.

Anyway, I think I'll try to get my PR merged as is, and try to explore further in the next weeks. It'd be awesome if you could investigate a little, but I understand this would reequire time and we all have things to do, so no worries!

Thanks for the support! 😄

[deivid-rodriguez]

Hi @mrcasals!

Long time no see, I hope everyone in the decidim world is doing fine in these odd days :)

In case it rings any bells, I remember configuring simplecov for decidim and having trouble with:

• SimpleCov.root getting a incorrect value. I think this was related to the PWD and using relative paths, so I tend to remember explicitly setting it to the absolute root path of the repo.
• Coverage reports overwriting each other. I remember fixing this by explicitly setting a different SimpleCov.command_name for each subrepo, so that they are properly merged instead of overwritten.

Probably not related to your current issue, but just in case.

Also, I'll try to find time for this myself too!

[PragTob]

Files shouldn't be reloaded unless someone is "evil". Evil in the sense of spring or something, or ActiveAdmin also used to hardcore load file. I mean, normally files should just be require'ed which shouldn't cause any duplicate loading as ruby checks that.

One of my favorite methods is good old puts debugging where I just put puts statements at the beginning of the files to see when/how they are loaded.

[mrcasals]

Mh, we don't use spring AFAIK, and we don't use ActiveAdmin... I'll try with the puts-based debugging, see if I get something, thanks! 😄

[deivid-rodriguez]

Hei @mrcasals!

This is probably not related, because I believe decidim is still on ruby 2.5, but I was also going nuts because of stuff being clearly misreported as uncovered on my library, and it ended up being a regression in ruby's coverage library.

I reported it here: https://bugs.ruby-lang.org/issues/16776.

Letting you know anyways just in case, because I remember using a newer ruby than I was supposed to while developing decidim locally 😅.

[deivid-rodriguez]

So, after getting a reply in there, I'd say it's not related to your problem.

At first I thought it could because Rails 6 uses a new code loader, zeitwerk, which does use some TracePoint hooks (which is what the coverage library no longer supports). But decidim does not yet support Rails 6, so I'd say it's not it.

[PragTob]

Wow that's some interesting interaction right there.

[mrcasals]

This is probably not related, because I believe decidim is still on ruby 2.5

@deivid-rodriguez exactly, it's still using a 2.5.x Ruby version, and no support for Rails 6 yet (it's on Rails 5.2.x)... I've double-checked, and both my dev machine and the CI use the correct Ruby version.

One solution is "add tests for each module so code is not tested through other modules", but that'd take quite a lot of time, and for specifics on how Decidim is organized we don't have much work-time to invest on this.

Is there any way I can help on debugging this? I'm pretty sure it might be an issue on our side, but I'd like to understand what's going on 😄

[deivid-rodriguez]

Hi @mrcasals!

I'd like to find some time to investigate myself, but... I'm not too positive that will happen 😅.

My suggestion for debugging this is to try to progressively eliminate stuff from the picture. Reproduce it running a single spec, start removing requires, try to get a repro directly without using RSpec... and so on, until you can at least figure out a clear culprit and that will hopefully give more clues at least on where to report it.

That's how I isolated https://bugs.ruby-lang.org/issues/16776, which at first I thought it was a simplecov issue, but it ended up being a problem with ruby's coverage library.

[PragTob]

Hey everyone,

I'm not too sure I'll get to it either although I recently decided I'll do more Open Source again. Albeit, this week I'm trying to run the first ever remote Ruby User Group Berlin so I might be biting off more than I can chew :)

[mrcasals]

@PragTob @deivid-rodriguez first, thanks for your time on this issue and open-source in general. It's always been a pleasure working with you, either directly or through your libs, so thank you very much!

I know this issue might be complicated, and maybe it's not even a problem with simplecov but a configuration issue on our repo, and due to Decidim's size it might be difficult to debug, so don't worry if you don't have time for this, I completely understand! 😄

Also, thanks for your ideas! I'll try to find some time to debug what's going on, maybe even try to replicate it on a smaller scale 😄

[deivid-rodriguez]

Sooo, I had a look at this today and this is a bug in simplecov. Fix should be simple, I'll create a PR tomorrow!

[mrcasals]

OMG @deivid-rodriguez thank you so much for taking the time to look at this! ❤️

[deivid-rodriguez]

@mrcasals No problem! #894 should do the trick!

[PragTob]

Thanks a bunch @deivid-rodriguez 💚

[PragTob]

@mrcasals if you could confirm whether this really fixes your problem, that'd be great! I might also consider cutting a release soon.

[mrcasals]

@PragTob I haven't tried it myself, but it seems like it's working properly! 😄 See decidim/decidim#6230 (comment)