From: eregontp@... Date: 2016-04-06T14:18:23+00:00 Subject: [ruby-core:74830] [Ruby trunk Bug#12220] Why does Coverage keep previously-loaded files as empty arrays? Issue #12220 has been updated by Benoit Daloze. Thank you for your reply. Xavier Shay wrote: > Hello, I filed #4796. It was a long time ago, and I don't remember what I was trying to do. Likely something around a "coverage per test" feature. > > > Right, there is a compatibility concern, and I would like to check simplecov about it. > > it does prevent to make accurate tests without additional efforts (filtering out the empty arrays). > > I don't feel making it "easier to write specs" is sufficient reason to change this, Please read my argument fully, following that is: "And lack of these tests made such a strange feature mostly hidden for everyone, lost the true purpose of the change, and do not document the expected behavior clearly." So this means, that up to this day, nobody understands this feature wells, and good tests would uncover that and most likely cause us to discuss it. So that's what I am trying to do, get Coverage tested properly so that other implementations can be compatible. Is that not an important concern? > but if simplecov test suite passes with it (pretty sure it doesn't rely on restartable coverage) then I don't have a strong objection. SimpleCov tests pass with this modification, and indeed they do not use restartable coverage (spec/ does not use Coverage.result, and Cucumber features spawn different processes). If some code used Coverage before SimpleCov starts to use it, then SimpleCov would get empty arrays from Coverage.result. This could have an impact for instance on add_not_loaded_files, which could report files as empty instead of not covered. The merging results feature seems like it would be unaffected due to its implementation: `new_resultset[filename] = (self[filename] || []).extend(SimpleCov::ArrayMergeHelper).merge_resultset(hash[filename] || [])` Another implementation could skip the merging if only one side exists, and empty arrays could then cause additional processing. I think this is a good enough example that the current behavior of restartable Coverage is harmful and unhelpful. As it is rarely used in practice it was not yet reported but nevertheless it is worthy to fix, if only to not leak this implementation detail bug to other implementations and future libraries using Coverage. ---------------------------------------- Bug #12220: Why does Coverage keep previously-loaded files as empty arrays? https://bugs.ruby-lang.org/issues/12220#change-57955 * Author: Benoit Daloze * Status: Open * Priority: Normal * Assignee: Benoit Daloze * ruby -v: ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux] * Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN ---------------------------------------- For instance, ruby -e 'require "coverage"; Coverage.start; require "set"; Coverage.result; Coverage.start; require "tmpdir"; p Coverage.result' {".../lib/ruby/2.2.0/set.rb"=>[], ".../lib/ruby/2.2.0/tmpdir.rb"=>[nil, nil, nil, nil, nil, nil, 1, 1, ...]} So Coverage.result stops coverage but also does some cleaning up. I think the most intuitive would be that the coverage Hash would be reset to be empty, but instead it's filled with empty arrays. As an example, this makes it fairly awkward to test and it enforces irreversible global state (https://github.com/ruby/spec/pull/219). I also do not see how this would be useful for coverage libraries. Is there a reason for this behavior? If not, would it be OK to clear the Hash when calling #result? ---Files-------------------------------- 0001-ext-coverage-coverage.c-Fully-reset-coverage-to-not-.patch (5.14 KB) -- https://bugs.ruby-lang.org/ Unsubscribe: