date_rolling_log_file: compare dates with != instead of >.

[wwood]

Hi,

lumberjack has been packaged in the GNU Guix package manager, where we run unit tests, and I came across this issue.

/gnu/store/ippi1rw3869rzv21v3ixvzrim40r2s02-ruby-2.2.3/bin/ruby -I/gnu/store/8mp9yjk9ij8ypck9mq8cl09fsvz2ps1p-ruby-rspec-support-3.2.2/lib/ruby/gems/2.2.0/gems/rspec-support-3.2.2/lib:/gnu/store/gigv44i4x36d3fr8wh578vxv6wf077yx-ruby-rspec-core-3.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.2.3/lib /gnu/store/gigv44i4x36d3fr8wh578vxv6wf077yx-ruby-rspec-core-3.2.3/lib/ruby/gems/2.2.0/gems/rspec-core-3.2.3/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
.F.............................................................................................

Failures:

  1) Lumberjack::Device::DateRollingLogFile should roll the file weekly
     Failure/Error: File.read("#{log_file}.#{today.strftime('week-of-%Y-%m-%d')}").should == "test week one#{Lumberjack::LINE_SEPARATOR}"
     Errno::ENOENT:
       No such file or directory @ rb_sysopen - /tmp/nix-build-ruby-lumberjack-1.0.9.drv-0/gem/spec/tmp/b731195059.log.week-of-2016-01-02
     # ./spec/device/date_rolling_log_file_spec.rb:45:in `read'
     # ./spec/device/date_rolling_log_file_spec.rb:45:in `block (2 levels) in <top (required)>'

This issue has been fixed in the attached pull request.

I also encourage you to make the tests not rely on the current time/date and instead use stubs. This helps makes the process of building and testing deterministic, which is important for reproducibility.

Thanks
ben

[bdurand]

Thanks. I'm keeping the specs dependent on the current date since this was an actual bug in the weekly date rolling and not just a problem with the specs. I did add a dependency on timecop, though.

[wwood]

Thanks. I'm keeping the specs dependent on the current date since this was an actual bug in the weekly date rolling and not just a problem with the specs.

Hard to argue against a test that picked up a bug. Just so long as it works I guess.

I did add a dependency on timecop, though.

I'm not sure what you were attempting to convey here, but adding dependencies is not music to the ears of a packager..

[bdurand]

The additional dependency is a development dependency just for running the specs. There are no additional runtime dependencies.

_____________________________

From: Ben J Woodcroft notifications@github.com
Sent: Saturday, January 2, 2016 4:36 PM
Subject: Re: [lumberjack] date_rolling_log_file: compare dates with != instead of >. (#26)
To: bdurand/lumberjack lumberjack@noreply.github.com
Cc: Brian Durand brian@embellishedvisions.com

Thanks. I'm keeping the specs dependent on the current date since this was an actual bug in the weekly date rolling and not just a problem with the specs.

Hard to argue against a test that picked up a bug. Just so long as it works I guess.

I did add a dependency on timecop, though.

I'm not sure what you were attempting to convey here, but adding dependencies is not music to the ears of a packager..

—
Reply to this email directly or view it on GitHub.

[wwood]

The additional dependency is a development dependency just for running the specs. There are no additional runtime dependencies.

Sure, I'm glad users aren't affected. But even a dev dependency means that timecop now has to be packaged, to run lumberjack's tests. And then timecop's dependencies, and so on. One wonders why I enjoy packaging..

The main difficulty with Ruby is that rubygems doesn't ask for tests to be included in the gem (though many are), and does not provide any link to the source code where such tests might be. So automating the process of packaging with tests is not possible. And as this issue shows, tests can be useful.

But anyway, beyond the rant, it's OK, timecop is a reasonably popular so should be packaged anyway. Thanks for pushing a new version btw.