Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add_group() sees files in 0.17.0 but does not in 0.18.1 #860

Open
JonJagger opened this issue Feb 12, 2020 · 31 comments
Open

add_group() sees files in 0.17.0 but does not in 0.18.1 #860

JonJagger opened this issue Feb 12, 2020 · 31 comments
Labels

Comments

@JonJagger
Copy link

I'm running some Ruby tests inside a docker container (Alpine 3.11)
The version of ruby inside the container is 2.6.5p114
My code is in a dir called /app (inside the image).
My tests are in a dir called /test (volume-mounted).
Note: the tests are not underneath the /app dir (and that is how I prefer it).

I get coverage of both the code (in /app) and the tests (in /test).
My coverage is setup like this..

require 'simplecov'

SimpleCov.start do
  filters.clear
  #add_group('debug') {|src| puts src.filename; false; }
  add_group('app') { |src| src.filename =~ %r"^/app/" }
  add_group('test') { |src| src.filename =~ %r"^/test/.*_test\.rb$" }
end
SimpleCov.root('/app')
SimpleCov.coverage_dir(ENV['COVERAGE_ROOT'])

My Gemfile.lock shows

simplecov (0.17.0)

All works fine.
I get coverage of the code.
I also get coverage of the tests (easy to keep this at 100%!)
:-)

When I move to simplecov 0.18.1 and leave everything else the same I am no longer getting any coverage of my tests (in /test). Uncommenting the line

  #add_group('debug') {|src| puts src.filename; false; }

shows that no /test/files are being seen at all.

@JonJagger JonJagger changed the title In 0.18.1 no longer get coverage (of the tests) In 0.18.1 I no longer get coverage (of the tests)...? Feb 12, 2020
@JonJagger JonJagger changed the title In 0.18.1 I no longer get coverage (of the tests)...? add_group() sees files in 0.17.0 but does not in 0.18.1 Feb 12, 2020
@PragTob
Copy link
Collaborator

PragTob commented Feb 12, 2020

👋

Hi @JonJagger !

Thanks for your report!

I have a hunch that this might be due to you calling SimpleCov.root("/app") - could you try running it without this please?

I'm guessing that with filters.clear you want to remove it and maybe that doesn't work anymore.

One more thing, you should be able to call both root and coverage_dir within the SimpleCov.start block!

@JonJagger
Copy link
Author

🤝
Hi
Thanks for the tips!

With 0.17.0, this works:

require 'simplecov'
SimpleCov.start do
  filters.clear
  #root('/app')
  coverage_dir(ENV['COVERAGE_ROOT'])
  add_group('debug') { |src| puts src.filename; false }
  add_group('app') { |src| src.filename =~ %r"^/app/" }
  add_group('test') { |src| src.filename =~ %r"^/test/.*_test\.rb$" }
end

It sees both /app/... and /test/...
Note I have commented out the call to root('/app')
Note it sees only /app/... if I comment out the line filters.clear

Switching to 0.18.1
It sees only /app/...
And it still sees only /app/... if I comment out the filters.clear

Thanks.

@PragTob
Copy link
Collaborator

PragTob commented Feb 12, 2020

@JonJagger hm that's... interesting. And weird. I have no idea how/why that is happening 🤷‍♂️

How do you run your tests? Aka what command is used? Where is the test setting invoked?

You don't happen to have a small repro reproducing it? I can try to reproduce it, but I fear I'll be out of luck because if we generally skipped test/ directories I think it'd show up somewhere else in our tests :|

e.g. https://github.com/colszowka/simplecov/blob/master/features/test_unit_basic.feature specifically tests they show up.

@JonJagger
Copy link
Author

I will try to put together a small repo that reproduces the problem...
But not today.

@PragTob
Copy link
Collaborator

PragTob commented Feb 12, 2020

Sure, no hurry. Would be great if you did, otherwise reproduction is sadly quite hard :(

If you want to be extra cool, there's a test_projects part of our repo where we put such repos and write cukes against. But I can also just copy a repro repo in there :)

@JonJagger
Copy link
Author

Had an unexpected few hours this evening...
https://github.com/JonJagger/possible-simplecov-bug

@PragTob
Copy link
Collaborator

PragTob commented Feb 13, 2020

@JonJagger thank you! 💚

For reference, you didn't manage to reproduce it without docker did you? I'll try without first and then well install docker and do that dance when I got time :)

@JonJagger
Copy link
Author

JonJagger commented Feb 13, 2020

No. I do everything inside docker nowadays. It is awesome.
If you want any help/advice re docker I'm happy to provide it.
By the way, I have a suspicion the problem is two sibling dirs directly off the root dir /
Cheers
Jon

@JonJagger
Copy link
Author

@PragTob I'm still really hoping we can resolve this error together somehow. It is usually very easy to install docker. Once you've done that you'll be able to follow the readme on the repo I created: Run one script. Edit one file. Run the script again. That way you would at least have replicated the error :-)

@PragTob
Copy link
Collaborator

PragTob commented Jun 23, 2020

@JonJagger hey, yeah I know and have used docker but my experiences have been.. mixed. Just didn't have it on my desktop before, but Corona has changed that as well. I should be fine. I guess I'll have to put together a day/half a day one of these days to iron out some issues and make a new release. Still feel free to ping me again if I haven't in the next ~2 weeks 🤞 (albeit life is also relatively hectic these days)

@JonJagger
Copy link
Author

Will do. Thanks :-)

@PragTob
Copy link
Collaborator

PragTob commented Jul 5, 2020

@JonJagger hey I tried debugging it but I believe the shell script is mac specific (at least it fails for me), at least I don't have an ip_address command.

However, while trying to then run it myself I noticed that the app with the Gemfile is in a separate folder from the test directory and a coverage_dir is set. This might be related to what was fixed in #894 - could you try running against the main branch again please?

@PragTob PragTob added the Bug label Jul 5, 2020
@JonJagger
Copy link
Author

Hi Tobias. Got your comment. Ah rats. A missing bash source command :(
Will try it some time in the coming week.
Thanks

@JonJagger
Copy link
Author

I ran it again. Same result.
0.17.0 gives me coverage for both tests and code.
0.18.5 gives me coverage only for code.
:(

@JonJagger
Copy link
Author

Hmmm. The reason for you getting a no ip_address failure is not a missing source command. The ip_address function is right there in the build_test.sh script. Odd? There is one thing it might be. I've edited the script. Could you pull again and retry. If it still fails could you add a 'set -x` after the first #line and run it again (to get a debug output).
Thanks

@PragTob
Copy link
Collaborator

PragTob commented Jul 6, 2020

@JonJagger will try later. But it's not released yet, so do gem 'simplecov', github: 'colszowka/ simplecov' that should pull the newest version from git :)

@JonJagger
Copy link
Author

@PragTob Alas adding the github: option to my Gemfile did not work. The output included this...

GIT
  remote: https://github.com/colszowka/simplecov.git
  revision: 9056bd3852d0daca2221c9b0a964b8abada1c48c
  specs:
    simplecov (0.18.5)
      docile (~> 1.1)
      simplecov-html (~> 0.11)

but require 'simplecov' is now failing.
Shelling into a container built from the image and running:
gem list simplecov
Prints only simplecov-html (0.12.2)
Running:
ruby -e 'require "simplecov"'
Prints

Traceback (most recent call last):
	2: from -e:1:in `<main>'
	1: from /usr/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require'
/usr/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require': cannot load such file -- simplecov (LoadError)

Ach so close!

@PragTob
Copy link
Collaborator

PragTob commented Jul 6, 2020

@JonJagger it's not there as a normal gem. Try bundle exec around everything i.e. the ruby require call or the test execution etc. That should work :)

@JonJagger
Copy link
Author

I added the 'bundle exec' to the test-script execution (and pushed it to the repo). That gets me the gem :-)
Alas the same problem still exists :-(

@PragTob
Copy link
Collaborator

PragTob commented Jul 7, 2020

@JonJagger ah damn :(

script still doesn't work for me as well :( DOCKER_MACHINE_NAME is unbound

Successfully built 440d66a4510e
Successfully tagged cyberdojo/bug:latest
+ container_up
+ printf '\n'

+ docker container rm test-bug-server --force
test-bug-server
+ docker run --detach --init --name test-bug-server --publish 4567:4567 --tmpfs /tmp --user nobody --volume /home/tobi/github/possible-simplecov-bug/test:/test:ro cyberdojo/bug
f33ad36769a10ca57324ec53ecd25fbca9a150d162627286c9410b450dea370c
+ wait_until_ready
+ local -r name=test-bug-server
+ printf 'Waiting until test-bug-server is ready'
Waiting until test-bug-server is ready+ local -r max_tries=20
++ ip_address
./build_test.sh: line 27: DOCKER_MACHINE_NAME: unbound variable
+ local -r my_ip_address=
++ seq 20
+ for _ in $(seq ${max_tries})
+ curl_ready
./build_test.sh: line 62: 1: unbound variable

@JonJagger
Copy link
Author

Ah that's a different error. Sorry. I've pushed a fix. Thanks.

@JonJagger
Copy link
Author

Hi @PragTob
gentle nudge... Still really hoping to fix this problem. Without a fix I am stuck at 0.17.0 and I really want to add branch coverage to my repos :-)

@PragTob
Copy link
Collaborator

PragTob commented Jul 23, 2020

@JonJagger 👋 thanks for the nudge.

Script's still not working for me 😅 I already added -f to the last rm because on first run the index.html isn't there but even then:

Waiting until test-bug-server is ready....OK
==================================
Running server tests
==================================
`/` is not writable.
Bundler will use `/tmp/bundler20200723-16-1ahgoe816' as your home directory temporarily.
Run options: --seed 34397

# Running:

.

Finished in 0.000453s, 2208.3879 runs/s, 2208.3879 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
/app/src/creator.rb
/test/server/creator_test.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/assertions.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/autorun.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/expectations.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/mock.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/parallel.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/pride_plugin.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/spec.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb
/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/unit.rb
/usr/local/lib/ruby/2.7.0/delegate.rb
/usr/local/lib/ruby/2.7.0/mutex_m.rb
/usr/local/lib/ruby/2.7.0/optparse.rb
/usr/local/lib/ruby/2.7.0/tempfile.rb
Coverage report generated for MiniTest to /tmp/reports. 1037 / 2078 LOC (49.9%) covered.
Coverage files copied to test/server/reports/
Couldn't get a file descriptor referring to the console

@PragTob
Copy link
Collaborator

PragTob commented Jul 23, 2020

So I tried running it without the script etc. and replicating what the script does but simplified. So I created test/tests.rb with:

require_relative 'coverage'

require_relative 'server/creator_test'

Running it still errors out:

tobi@speedy:~/github/possible-simplecov-bug(master)$ bundle exec ruby test/tests.rb 
Run options: --seed 9406

# Running:

.

Finished in 0.000415s, 2409.0697 runs/s, 2409.0697 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/delegate.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/mutex_m.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/optparse.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/tempfile.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/2.7.0/tmpdir.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/assertions.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/autorun.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/expectations.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/mock.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/parallel.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/pride_plugin.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/spec.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/test.rb
/home/tobi/.asdf/installs/ruby/2.7.0/lib/ruby/gems/2.7.0/gems/minitest-5.14.1/lib/minitest/unit.rb
/home/tobi/github/possible-simplecov-bug/app/src/creator.rb
/home/tobi/github/possible-simplecov-bug/test/server/creator_test.rb
Coverage report generated for Unit Tests to /home/tobi/github/possible-simplecov-bug/coverage. 1046 / 2124 LOC (49.25%) covered.

But even with 0.17.0 the tests group is empty.... however it seems the file is seen 🤷

When I use it from master, the file is seen but also doesn't show up in the category (I guess the regex is wrong):

tobi@speedy:~/github/possible-simplecov-bug(master)$ bundle exec ruby test/tests.rb 
Run options: --seed 15482

# Running:

.

Finished in 0.000390s, 2564.7602 runs/s, 2564.7602 assertions/s.

1 runs, 1 assertions, 0 failures, 0 errors, 0 skips
/home/tobi/github/possible-simplecov-bug/app/src/creator.rb
/home/tobi/github/possible-simplecov-bug/test/server/creator_test.rb
Coverage report generated for Unit Tests to /home/tobi/github/possible-simplecov-bug/coverage. 10 / 10 LOC (100.0%) covered.

So I can't detect anything wrong right now.

@JonJagger
Copy link
Author

JonJagger commented Jul 23, 2020

Ok. We are close now! (I think)
If you look at the output (from two messages ago) you'll see you got...

/app/src/creator.rb
/test/server/creator_test.rb

This is from test/server/coverage.rb which has a debug group just to print all filenames being seen.
This shows with 0.17.0 you see a file from /app/ and a file from /test/.
Getting the coverage's index.html out seems to be failing but I guess that's not really needed so I've commented that out.
I've also changed the user to root in the build script and that seemed to make a difference.
So if you could pull from the repo again and follow the README.md one more time hopefully you will get to see that you dont get

/test/server/creator_test.rb

when Gemfile is set to 0.18.5

Thanks. I really appreciate your patience on this.

@PragTob
Copy link
Collaborator

PragTob commented Jul 23, 2020

alrighty, confirmed it doesn't show up anymore. I'm inclined to believe it might be the default root_filter we started including. Pkay nah that was always there. strange.

@JonJagger
Copy link
Author

When I comment out the filters.clear I get the same behaviour on both 0.17.0 and 0.18.5
...neither show /test/ file.

@PragTob
Copy link
Collaborator

PragTob commented Jul 26, 2020

Alright. So this is fun and broken on many different levels 🤔

  1. Files get stuck in UselessResultsRemover which snuck in during the big branch coverage PR, not sure why it was added anymore. Big PRs, hey?
  2. without fitlers.clear it also gets stuck in the root_filter
  3. Now you could simply set root to "/" but both root filter and useless results remover force append a file separator and well // doesn't match any paths anymore. While File.expand_path("") turns out to be the current directory.
  4. simple setting root to "/" also fails because we default project_name to the base name of the last split of the root both (see project_name method)

Future fix:

  • set root correctly to "/"

Current possible workarounds (or so I believe):

  • Moving test within app/on one level of the Gemfile should fix it automagically (it's very uncommon in Ruby not to have the test directory be a sub directory of the directory with Gemfile)
  • Putting all code within another directory like "/foo" and setting the root to that directory should also solve it

Simplecov fixes for now:

  • figure out UselessResultsRemover vs. root filter duplication and probably get rid off the UselessResultsRemover
  • Fix it so that root can be set to "/" and the aforementioned filter will work, also that project_name will work when set like that

@JonJagger
Copy link
Author

Ok. That's for the report. I will ponder what to do. It should be relatively painless to docker volume-mount the tests to a different directory.
Re your two current possible workarounds... do I need both of them? or should either one be ok by itself?

@PragTob
Copy link
Collaborator

PragTob commented Jul 26, 2020

@JonJagger each one should work by itself, all granted my understanding of the problem is complete.

Virtually every ruby project I've ever seen goes:

project_name

  • app
  • config
  • lib
  • spec | test
  • Gemfile

so all in one, that's what I'd recommend.

All in all, it's also all bugs and I'm somewhat confident I'll fix and release them within the next 3 weeks.

@JonJagger
Copy link
Author

@PragTob I'll hang on for a root = '/' fix.
Re the Ruby project dir/ structure. Agreed. In Docker the app dir often seems to be /app/ and there is no dir named on the project at all. So there is perhaps some tension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants