Display annotations in coffee files #6519

Merged
merged 2 commits into from May 30, 2012

Projects

None yet

3 participants

@oscardelben

Cherry picked as discussed here

cc @carlosantoniodasilva

P.S. was not able to run the test suite, trying to figure it out.

@carlosantoniodasilva
Ruby on Rails member

Apparently the test is failing on 3-2 running on 1.9 (didn't test under 1.8):

$ be ruby -Irailties/test railties/test/application/rake/notes_test.rb
Run options: 

# Running tests:

F

Finished tests in 4.831979s, 0.2070 tests/s, 2.4835 assertions/s.

  1) Failure:
test_notes(ApplicationTests::RakeTests::RakeNotesTest) [railties/test/application/rake/notes_test.rb:43]:
<4> expected but was
<3>.

Whereas in master it's all fine:

$ be ruby -Irailties/test railties/test/application/rake/notes_test.rb
Run options: --seed 39373

# Running tests:

...

Finished tests in 15.901273s, 0.1887 tests/s, 4.4022 assertions/s.

Do you mind taking a look what might be going on? Thanks.

@oscardelben

I got that failing test too, but I don't understand why it's testing that the length of each line should be 4 characters.

Right now the lines look like:

["  1", "1000", "  1", "  1", "  1"]

I think some new formatting was added to Rails edge that made this test pass, so if it's ok for you I'll remove that assertion from 3.2. Everything else seems fine.

@carlosantoniodasilva
Ruby on Rails member

Yeah, looks like there are commits to align rake notes in master: 9299bfd and c5f4b20. So I believe it's fine to just kill these assertions, as 3-2 doesn't have this logic. Thanks!

@oscardelben

Done, now the test pass for me.

@carlosantoniodasilva carlosantoniodasilva and 1 other commented on an outdated diff May 29, 2012
railties/test/application/rake/notes_test.rb
assert_match /note in erb/, output
assert_match /note in haml/, output
assert_match /note in slim/, output
+ assert_match /note in ruby/, output
+ assert_match /note in coffee/, output
+
+ assert_equal 5, lines.size
@carlosantoniodasilva
carlosantoniodasilva May 29, 2012

I think we can just kill this assertion together with the lines variable, it doesn't test anything important here.

@carlosantoniodasilva carlosantoniodasilva and 1 other commented on an outdated diff May 29, 2012
railties/test/application/rake/notes_test.rb
Dir.chdir(app_path) do
output = `bundle exec rake notes`
-
+ lines = output.scan(/\[([0-9\s]+)\]/).flatten
@carlosantoniodasilva
carlosantoniodasilva May 29, 2012

Sorry I didn't make it clear, by lines variable I meant this line :)

@oscardelben
oscardelben May 29, 2012

Ok, will do later today

@oscardelben

I think it's ok now.

@carlosantoniodasilva
Ruby on Rails member

@oscardelben yeah, looks fine to me. Thank you :)

@carlosantoniodasilva carlosantoniodasilva merged commit f09ae85 into rails:3-2-stable May 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment