YahooArchive / arrow

FE Test framework designed to promote TDD
http://yahoo.github.io/arrow/arrow_intro.html
BSD 3-Clause "New" or "Revised" License
55 stars 59 forks source link

A space after a comma in the 'commonLib' string in 'test-descriptor.json' causes a fail #206

Closed nottoseethesun closed 10 years ago

nottoseethesun commented 10 years ago

commonLib: "foo.js, bar.js", fails where commonLib: "foo.js,bar.js", succeeds.

However, it is common practice to allow a space in a comma-delimited list, for human readability. The fail is a failure to load the files specified.

commonLib: "foo.js, bar.js", should succeed.

pranavparikh commented 10 years ago

@christopherbalz Thanks for reporting this. I'll take a look.

nottoseethesun commented 10 years ago

Welcome - I realized this happens under an old version ( 0.0.84, I believe), so perhaps it's been fixed by now. But if not, it would be really good to fix.

redonkulus commented 10 years ago

Furthermore @pranavparikh, is there any reason it can't be an array of libraries instead of comma separated string?

pranavparikh commented 10 years ago

@redonkulus , There's no reason for it. I fully support having an array instead of comma separated string ( supporting both though for backward compatibility), wherever there is such case. We'll take it up as enhancement. Thanks for the feedback.

redonkulus commented 10 years ago

@pranavparikh is this the code that needs to change? https://github.com/yahoo/arrow/blob/e42b9d49385463ad641c51dc602c638cd59cf923/lib/util/libmanager.js#L29

redonkulus commented 10 years ago

ping @pranavparikh @proverma

pranavparikh commented 10 years ago

@redonkulus ,

Trimming each element of the array "arrLib" before processing should work. I'll test it out and make the change.

https://github.com/yahoo/arrow/blob/e42b9d49385463ad641c51dc602c638cd59cf923/lib/util/libmanager.js#L30

pranavparikh commented 10 years ago

https://github.com/yahoo/arrow/pull/233 should fix this issue

pranavparikh commented 10 years ago

The fix is out with 0.5.3.

234 - Created a separate issue ( enhancement) for allowing array for params like commonLib