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

Document how to test #171

Closed
wants to merge 1 commit into from
Closed

Document how to test #171

wants to merge 1 commit into from

Conversation

bf4
Copy link
Contributor

@bf4 bf4 commented Feb 6, 2015

Related to work on #134 and #151

### Installing the Rubies

For setting up a dev environment with all the Rubies on it, it is convenient to
use the [rake-compiler-dev-box](https://github.com/tjschuck/rake-compiler-dev-box).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nothing we've ever used thus I wouldn't feel comfortable including it in our readme.

Copy link
Contributor Author

@bf4 bf4 Feb 6, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally use chruby and ruby-install. There are many options, though -- rvm is still a widely used option as is rbenv/ruby-build.

Overall, we're agnostic to what kinds of tools contributors use (and I imagine not everyone on the RSpec core team uses chruby/ruby-install like I do), but like @JonRowe said I don't feel comfortable having a recommendation in the README that none of us core devs have used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I use RVM

@bf4
Copy link
Contributor Author

bf4 commented Feb 6, 2015

Is there anything salvageable from this PR? caveats about the difference in what travis does, and how this may fail your build? e.g. rubocop

@myronmarston
Copy link
Member

Is there anything salvageable from this PR? caveats about the difference in what travis does, and how this may fail your build? e.g. rubocop

I think it's quite useful to include more documentation about how to contribute to RSpec but I'm not sure this is the best place for it (besides the encoding-specific stuff this gem deals with). What do you think about adding more to http://rspec.info/contributing/ ? If you want to contribute to that, the repo is https://github.com/rspec/rspec.github.io.

@bf4
Copy link
Contributor Author

bf4 commented Feb 6, 2015

Ok. Just trying to lower the burden for others as developing has been different from other gems

@myronmarston
Copy link
Member

Ok. Just trying to lower the burden for others as developing has been different from other gems

Thanks, we do appreciate it! We just want to make sure that the contributing recommendations are things that core RSpec developers actually use and recommend.

Also, since you've been working in the quagmire of encodings, I think your experience is pretty atypical: encoding issues have their own, unique set of problems that most contributors don't face.

@JonRowe
Copy link
Member

JonRowe commented Feb 6, 2015

I will mention that rake actually provides a very close pass to what Travis does (in that it runs rspec, cucumber and rubocop) maybe that's worth mentioning?

@bf4
Copy link
Contributor Author

bf4 commented Feb 8, 2015

@JonRowe you mean just bundle exec rake? I don't see how that would run the cucumber or rubocop. They're available as rake tasks, but not part of the default. (confirmed via rake -W default) I would think running script/run_build (which does run them) would be a better choice. (Also, for me, rake fails as it outputs warnings to stderr. You might want to filter out warnings that originate outside of the project)

I'm not a maintainer, but it bothers me that there's not more dev/ci parity in how you run your tests. At the very least, I think it should say that since CI will test 1.8.7, 1.9.2, 1.9.3, and 2.x, 'you may want to test on those as well if you think your changes may behave differently on the different versions.' And also to run 'rubocop lib' on MRI Ruby 1.9.3 or higher.

That said, I do agree that it's also a priority to lower the barrier to PR, so I'm happy to change this to whatever minimal test-running-code you'd recommend.

@JonRowe
Copy link
Member

JonRowe commented Feb 8, 2015

@JonRowe you mean just bundle exec rake? I don't see how that would run the cucumber or rubocop. They're available as rake tasks, but not part of the default.

Ah support must be an odd one out, the other projects do run cucumber and rubocop as part of the default task.

@JonRowe
Copy link
Member

JonRowe commented Feb 8, 2015

I would think running script/run_build (which does run them) would be a better choice.

This is really intended to be Travis specific and does a lot more work than we would expect for short feedback (like checking across repos, and individual spec files)

I'm not a maintainer, but it bothers me that there's not more dev/ci parity in how you run your tests. At the very least, I think it should say that since CI will test 1.8.7, 1.9.2, 1.9.3, and 2.x, 'you may want to test on those as well if you think your changes may behave differently on the different versions.' And also to run 'rubocop lib' on MRI Ruby 1.9.3 or higher.

The point of CI is both to lower the barrier of entry and ensure compatibility without running it, people will quickly see if they do something that breaks an older version of Ruby, however if you'd like to add a paragraph explaining they should check Travis for those failures please do.

That said, I do agree that it's also a priority to lower the barrier to PR, so I'm happy to change this to whatever minimal test-running-code you'd recommend.

rake and we'll modify the default task to run rubocop too

@bf4
Copy link
Contributor Author

bf4 commented Feb 8, 2015

@JonRowe Fantastic, thanks! (Sorry if I'm being annoying. I like docs :).

### Running tests

`bundle exec rake` locally, and check the CI results in your pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see that a place for this yet exists on http://rspec.info/contributing/. If something like this text is ok, I would make a PR to add it to the page and link to it from here.

You can see what versions RSpec is tested against by looking at the
[.travis.yml](https://github.com/rspec/rspec-support/blob/v3.2.1/.travis.yml#L16-26)
(noting [allowed failures](https://github.com/rspec/rspec-support/blob/master/.travis.yml#L34-L36))
and the [appveyor.yml](https://github.com/rspec/rspec-support/blob/v3.2.1/appveyor.yml#L32-L34).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We support > 1.8.7 basically...

@JonRowe
Copy link
Member

JonRowe commented Feb 8, 2015

I'd actually rather this went into the contributing page, and we added a note onto our read mes about checking there. Seems easier than having to repeat this across all 4/5 repos...

@bf4
Copy link
Contributor Author

bf4 commented Feb 8, 2015

I'll make the PR to the contributing page, and hopefully it will look pretty. Should I include the encoding caveats? (minus the reference to the vagrant box)

@JonRowe
Copy link
Member

JonRowe commented Feb 8, 2015

You could include caveats about encoding although I think I'd like it reworded to be a bit more general.

@bf4
Copy link
Contributor Author

bf4 commented Feb 8, 2015

Ok, I think I'm just going to open a PR for discussion since I foresee some back and forth about where it should be on the site, how to format it, and wording

@bf4
Copy link
Contributor Author

bf4 commented Feb 8, 2015

rspec/rspec.github.io#65 if ok would direct me to add the docs to rspec-dev.

@fables-tales
Copy link
Member

Can we close this pull request? It looks like it's not in a state to merge and has been superseeded.

@bf4
Copy link
Contributor Author

bf4 commented Apr 10, 2015

Yeah

@bf4 bf4 closed this Apr 10, 2015
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.

4 participants