Fix environment issues for tests #19

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

stevenbenner commented Mar 22, 2013

Currently the tests for this task are very sensitive to environment and will always fail on Windows systems.

The tests for banner stripper have hard coded \n line endings, this means that Windows users who have not set their git global configuration to use UNIX line endings will see these tests fail.

On the other hand, Windows developers who have set their git global configuration for force UNIX style line endings will see the concatenation tests fail because the task uses grunt.util.linefeed for line endings, which defaults to \r\n on Windows systems. This means that the concatenated files will not match the LF-only fixtures pulled from the repo.

My suggestion:

  • Include a .gitattributes file with * eol=lf to force git to save pulled text files with UNIX style line endings.
  • Set grunt.util.linefeed = '\n' in the project's Gruntfile to override the default value in Windows environments.
  • Optional: Change the banner stripper tests to use grunt.util.linefeed instead of hard coded \n characters.

What are your thoughts?

stevenbenner added some commits Mar 22, 2013

Added .gitattributes file to force LF line endings.
This commit will force git to use UNIX style line endings in the working
directory.
Owner

shama commented Mar 22, 2013

Sounds good. Could you change the .gitattributes file to * text=auto instead. Also let's use grunt.util.normalizelf() to fix the linefeeds. Thanks!

Contributor

stevenbenner commented Mar 22, 2013

Actually now that you mention it, using grunt.util.normalizelf() across the board in the tests might make a lot more sense then what I'm doing here. Since the tests read the file into a variable we can easily normalizelf() on the temp/expected files before comparing them.

If we did that then there wouldn't be a need for the .gitattributes file or the grunt.util.linefeed override.

Also: from my understanding text=auto doesn't enforce any standards in the working directory, it just normalizes the line endings in text files to LF for saving them in the repo. Line endings in the working directory will still be whatever the default git configs say for the person pulling down the files (which would be fine if we use normalizelf() in the tests).

Contributor

stevenbenner commented Mar 22, 2013

@shama I've committed the changes you requested. Please let me know if this is what you're looking to see.

Two notes:

  • I added a helper function to the tests because repeating normalizelf(read(blah)) was nasty.
  • The testing revealed an actual bug: The line-comment regex was only searching for .*\n, which wouldn't match CRLF line endings. I've fixed that bug in this pull request so that this will pass testing in my environment. I apologize for adding an unrelated bugfix to this PR.
Contributor

stevenbenner commented Mar 31, 2013

@shama Just checking in. This one has been sitting around for a while, was there anything you wanted to see changed?

If you need a rebase just let me know.

Owner

shama commented Mar 31, 2013

Sorry for the delay! LGTM. I'll merge this shortly. I'm away from my office atm.

@shama shama closed this in aa6f423 Mar 31, 2013

Owner

shama commented Mar 31, 2013

Never mind, snuck away to get this merged. Thanks Steven!

Contributor

stevenbenner commented Mar 31, 2013

Thanks!

@stevenbenner stevenbenner deleted the stevenbenner:line-endings branch Mar 31, 2013

@ericclemmons ericclemmons referenced this pull request in ericclemmons/grunt-angular-templates Oct 16, 2013

Closed

Fix newlines in templateCache #51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment