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

Convert tests to RSpec #117

Merged
merged 63 commits into from
Nov 1, 2014
Merged

Convert tests to RSpec #117

merged 63 commits into from
Nov 1, 2014

Conversation

twe4ked
Copy link
Member

@twe4ked twe4ked commented Oct 10, 2014

No description provided.

twe4ked added 30 commits October 8, 2014 15:37
This test originally covered #86.
@twe4ked
Copy link
Member Author

twe4ked commented Oct 12, 2014

$ git diff master..rspec --stat
 .travis.yml          |    2 +
 Gemfile              |    4 +
 Gemfile.lock         |   35 +
 Makefile             |    2 +-
 spec/fresh_spec.rb   | 2095 ++++++++++++++++++++++++++++++++++++++++++++++++++
 spec/spec_helper.rb  |  214 ++++++
 spec/support/bin/git |   65 ++
 test/fresh_test.sh   | 2065 -------------------------------------------------
 test/support/shunit2 | 1048 -------------------------
 test/test_helper.sh  |  153 ----
 10 files changed, 2416 insertions(+), 3267 deletions(-)

Wow. Finished. Let me know what you think @jasoncodes.

@twe4ked twe4ked changed the title WIP: RSpec Convert tests to RSpec Oct 12, 2014
This was referenced Oct 12, 2014
@star-szr
Copy link

This looks really nice! I'm looking into adding test coverage to another shell script: https://github.com/virtualhost/virtualhost.sh.

Could you share your main reason(s) for switching to RSpec? I'm guessing one would be that shunit2 looks to be abandoned but I'm also wondering if you've found the experience to be a positive one. I've always loved that fresh has test coverage so I've been following this PR with great interest :)

@twe4ked
Copy link
Member Author

twe4ked commented Oct 18, 2014

@cottser There are a few reasons for changing. The first is simply for organisation, describe/context blocks mean we can group related tests together. I also find it easier to add helper methods to make the test readability better. Having access to thing like active support's strip_heredoc method is fantastic.

Another reason is the error messages we get. I find coloured diffs much easier to parse.

screen shot 2014-10-18 at 3 30 34 pm
screen shot 2014-10-18 at 3 31 43 pm

I also find RSpec's --color option really useful.

Ruby's system call is great and using active support to capture stdout/stderr worked well.

I'd recommend it if you're going to have more than a dozen tests. It was nice having the tests in the same language as the actual code but I think this is going to be easier going forward.

@star-szr
Copy link

Awesome, thank you very much for the response @twe4ked! I will be digging into this code in the near future to learn as much as I can.

@twe4ked
Copy link
Member Author

twe4ked commented Oct 18, 2014

No worries, run_fresh is probably the most interesting method. It's pretty horrible but it works quite well. The strange order of the assertions are so we get the best error messages. For instance if we expect stdout to be empty we check that before checking stderr and the exit code so we can see what we got in stdout. Not sure if that made sense.

Another two options to look at are bats and shpec. We decided against shpec because of this open issue: rylnd/shpec#15. I don't think we ever really looked into Bats properly.

@jasoncodes jasoncodes merged commit 9a350b9 into master Nov 1, 2014
jasoncodes added a commit that referenced this pull request Nov 1, 2014
Closes #117

* rspec: (75 commits)
  Ensure setting FRESH_NO_BIN_CHECK disables check
  Improve example names
  Reorder tests
  Colourful tests
  Always return a string from `expect_shell_sh`
  Prefer `unless foo` over `if !foo`
  Use Hash#fetch for default values
  Assert valid options keys to test helper methods
  Fix require name
  Add pry to assist with debugging
  Use `expect(pathname).to exist` over `expect(File).to exist(pathname)`
  Remove .gitignore
  Remove shUnit2 related files
  Convert _parse_fresh_dsl_args tests
  Convert private function tests
  Use FileUtils chmod method
  Convert subcommand tests
  Convert fresh-options tests
  Convert "edit" tests
  Convert tests for adding fresh lines interactively
  ...
@twe4ked
Copy link
Member Author

twe4ked commented Nov 21, 2014

@cottser Looks like there is some work happening on shpec that might be interesting. If they have an end function it may mean they can add the cleanup code.

rylnd/shpec#43

@star-szr
Copy link

Nice, thanks @twe4ked :)

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.

3 participants