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

Giving Roo's tests some love and attention #365

Merged
merged 9 commits into from
Dec 30, 2016

Conversation

stevendaniels
Copy link
Contributor

Contains the following changes:

  1. Refactored tests into different formats. (i.e. test_csv.rb, test_excelx.rb)
  2. Added rack app for testing download capabilities
  3. Fixed TestRoo#test_finalize

Refactored tests into different formats

This is the initial cleanup work for Roo's tests. Over the past year, I've
spent a lot of time thinking about #242, and I've written a lot of code
around implementing such an API. Unfortunately, I'd always end up
at a point where I wasn't happy with the either the test coverage of
the new API or the how the functionality between the two APIs was
mixed. As I said, I have a lot of partially written code.

The ultimate goal of the test refactoring is to allow me to create the
new API while also being able to take advantage of Roo's test suite.

Added Rack app for testing download capabilities

Added a rack app for downloading spreadsheet files. This allows Roo
to test downloading files without webmock or VCR.

###Fixed TestRoo#test_finalize

test_finalize broke in ruby 2.4 because of changes in Ruby's GC. In
Ruby 2.4, the last object being created didn't get GC'd, which was a
change from earlier behavior. The initial fix was to fork the process to
make sure directories where being cleaned properly.

Unfortunately, this caused a segfault when running the entire test suite.
The segfault issue was related to where ObjectSpace.define_finalizer
was being called from (a class that was being extended by a module).
This is probably related to Nokogiri, but I wasn't able to develop a trivial
example of the error.

Moving the finalizer's cration to Roo::Excelx/Roo::OpenOffice classes
fixed the issue in Ruby 2.4.0.

[Same as #361 which disappeared somehow]

+ Removed some commented out tests
+ Moved tests requiring downloading to format specific files
+ Updated gemspec and Gemfiles
Also did some minor test refactoring
`test_finalize` broke in ruby 2.4 because of changes in Ruby's GC. The
initial fix was to fork the process to make sure directories where being
cleaned properly.

Unfortunately, this caused a segfault when running the entire test suite.
The issue was related to where `ObjectSpace.define_finalizer` was being
called from (a class that was being extended by a module). The issue is
probably related to Nokogiri, but I wasn't able to develop a trivial
example of the error.

Creating the finalizer in the class fixed the issue in Ruby 2.4.0.
@coveralls
Copy link

coveralls commented Dec 30, 2016

Coverage Status

Coverage increased (+0.06%) to 93.813% when pulling 3690462 on feature/refactor_tests into 143fffa on master.

@stevendaniels stevendaniels merged commit 8824f8a into master Dec 30, 2016
@stevendaniels stevendaniels deleted the feature/refactor_tests branch December 31, 2016 01:19
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jan 8, 2017
## [2.7.1] 2017-01-03
### Fixed
- Fixed regression where a CSV's encoding was being ignored [372](roo-rb/roo#372)

## [2.7.0] 2016-12-31
### Fixed
- Added rack server for testing Roo's download capabilities [365](roo-rb/roo#365)
- Refactored tests into different formats [365](roo-rb/roo#365)
- Fixed OpenOffice for JRuby [362](roo-rb/roo#362)
- Added '0.000000' => '%.6f' number format [354](roo-rb/roo#354)
- Add additional formula cell types for to_csv [367][roo-rb/roo#367]

### Added
- Extracted formatters from Roo::Base#to_* methods [364](roo-rb/roo#364)

## [2.6.0] 2016-12-28
### Fixed
- Fixed error if sheet name starts with a slash [348](roo-rb/roo#348)
- Fixed loading to support files on ftp [355](roo-rb/roo#355)
- Fixed Ruby 2.4.0 deprecation warnings [356](roo-rb/roo#356)
- properly return date as string [359](roo-rb/roo#359)

### Added
- Cell values can be set in a CSV [350](roo-rb/roo#350)
- Raise an error Roo::Excelx::Extractor document is missing [358](roo-rb/roo#358)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants