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

Fix render tests if erubis is installed #44

Closed
wants to merge 1 commit into from

Conversation

jeremyevans
Copy link

Tilt automatically uses erubis to render erb if erubis is installed,
and the two handle whitespace slightly differently.

Tilt automatically uses erubis to render erb if erubis is installed,
and the two handle whitespace slightly differently.
@soveran
Copy link
Owner

soveran commented Jul 21, 2014

I think the goal of the tests is to verify that everything works with the minimum set of gems (those defined in .gems). As erubis is not listed, I think this test is more for Tilt itself, and shouldn't be included here. But I'd like to hear from @cyx, @frodsan and @djanowski too.

@cyx
Copy link
Collaborator

cyx commented Jul 21, 2014

I think having erubis is more of a special case, and also given our methodology of isolating gemsets via @soveran/gs then the scenario is not likely to happen in a test scenario.

@jeremyevans
Copy link
Author

The default makefile just runs cutest, it doesn't attempt to isolate gemsets. I agree that if you attempt to isolate gemsets to ensure that tilt will not pick up erubis automatically, it makes sense not to apply the patch. However, that doesn't seem to be the case currently, unless the recommended way to test it not via make test. In which case, the proper way to test cuba itself should be documented (apologies if it is already and I missed it)

@soveran
Copy link
Owner

soveran commented Jul 21, 2014

Good point, I just added a Contributing section to the README. In general, I think we assume no other gems are modifying the environment. The fact that erubis being installed changes the behavior of Tilt is sad.

@soveran soveran closed this Jul 21, 2014
@jeremyevans jeremyevans mentioned this pull request Jul 22, 2014
@jeremyevans jeremyevans deleted the render_test branch July 30, 2014 17:56
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.

None yet

3 participants