view · edit · sidebar · attach · print · history

<< | Index | CloneHttp >>

Setup

First, I am going to check out all relevant code into a single repository, mark a few interesting commits for later use and see if I can run the tests.

Cloning the repositories:

    git clone git://scm.ywesee.com/spreadsheet
    cd spreadsheet

    git remote add github-timon https://github.com/timon/spreadsheet.git
    git fetch github-timon master:t
    git remote add github-johanlunds https://github.com/johanlunds/spreadsheet.git
    git fetch github-johanlunds main_repo_add_colors_with_timon_master_and_dblock_gemified:j

Marking interesting commits:

    # list all the commits in 't' that are not in 'master'
    git cherry master t
    # 0386a7 is the first such commit
    git tag d 0386a7413726be5020e77f41362d4fd2c1a7f4b3~1
    # ... use gitk to make a tag named 't1' pointing to commit #3

Setting up the workspace and running the tests:

    git checkout -b work
    rvm --create use 1.8.7@spreadsheet
    gem install ruby-ole
    # ... uncomment regex fix in tests/suite.rb
    ruby -rubygems test/suite.rb

The plan

Since the goal is just to create the test cases, I will cherry pick the two commits, write the tests, and finally cherry pick the test commits into a separate branch/patch.

    git cherry-pick t1

Testing patch #3

To test the writer, I can use *merge_cells* in a new (or existing) spreadsheet, write it out, and see if we get the same result after reading.

To test the reader, I could either hope the same mistake wasn't made twice in the reader and writer (that is, the correct protocol is implemented), or add a new .xls file to the data directory. I've chosen to do the latter.

Testing patch #5

Patch #5 is buggy and makes a few existing tests fail. It moves out *@merged_cells* from *Worksheet* to *Excel::Worksheet*, making the assumption that *Excel::Writer* always works on *Excel::Worksheet*. This is not always true.

The only reason to move the *@merged_cells* instance variable from *Worksheet* to *Excel::Worksheet* was to use the *offset* class method. Exactly the same effect, in this case, can be achieved by replacing *attr_reader(:merged_cells)* with *attr_accessor(:merged_cells)* in *Worksheet*.

Because the patch is buggy, and implements only a trivial change (s/attr_reader/attr_accessor), I've decided to not write a test case just yet.

Publishing

To prepare for merging into master, I will remove the cherry-picked 't1' using interactive rebase, rename the branch 'work' and delete the new tags:

    git rebase -i master
    git branch -m merged_cells_tests
    git tag -d d t1

Now I can push the finished patch with test cases to github:

    git remote rename origin upstream
    git remote add origin git@github.com:mjane/spreadsheet.git
    git push origin master
    git push origin merged_cells_tests

Notes

  • The method *read_range_address_list* in biff5.rb is never used.
  • LibreOffice Calc forbids overlapping merged ranges, so that's probably illegal. This patch allows pretty much anything.
  • I had to read a cell (e.g. *sheet[0,0]*) to force parsing of the worksheet. This suggests that *merged_cells* accessor should call *ensure_rows_read* by itself.
view · edit · sidebar · attach · print · history
Page last modified on January 23, 2012, at 11:30 AM