Skip to content

Jts ct ruby specs#209

Closed
johnschoeman wants to merge 2 commits intoprettier:masterfrom
johnschoeman:jts-ct-ruby-specs
Closed

Jts ct ruby specs#209
johnschoeman wants to merge 2 commits intoprettier:masterfrom
johnschoeman:jts-ct-ruby-specs

Conversation

@johnschoeman
Copy link
Contributor

Commit 1:

Introduce spec helpers …
Why:
We would like to be able to have explicit tests that will allow to
ensure that prettier-ruby behaves as intended. Ideally these tests can
be run independently from the test suite and could allow for TDD.

This commit:
introduces an assert_equal_and_valid method which takes an input string
and an expected output string and will validate that prettier can be
applied to the expected string by:

writing the expected string to a Tempfile, 'prettier-ruby-test.rb',
applying prettier-ruby to the Tempfile with the 'bin/print' executable,
asserting with minitest that the resultant file is equal to the expected
string,
and executing the resultant file with Open3 to capture any stderr and
status to validate that the formatted code can be run without throwing
an error.

Two example specs are provided in this commit to demonstrate the spec
helpers usage

Co-authored-by: christoomey chris@thoughtbot.com
Co-authored-by: johnschoeman johnschoeman1617@gmail.com


Commit 2:

Fix block spacing bug …
Why:
Currently if a block is given just a parameter and no actual content the
formatter will return a block with an extra space, which is a rubocop
error: 'Layout/ExtraSpacing: Layout/ExtraSpacing: Unnecessary spacing
detected.'

for example:

{ |el| }

becomes

{ |el|  }

This commit:
Updates the block node to not add an extra space if the block has no
contents.

Co-authored-by: christoomey chris@thoughtbot.com
Co-authored-by: johnschoeman johnschoeman1617@gmail.com

@johnschoeman
Copy link
Contributor Author

Being able to write tests in this way offers a few advantages:

  1. being able to run a single test at a time.
  2. being able to apply prettier-ruby to the test suite itself.

To be clear, we see this as an addition to the existing testing format of the minitest file which is prettier'd and then run post-formatting.

@johnschoeman
Copy link
Contributor Author

Looks like nodes.test.js is failing the travis build. its not clear to me why this would be, since this pr doesn't add or remove any node types. Do you know what needs to be done to fix this @kddeisz ?

@kddnewton
Copy link
Member

Yeah they introduced a new node type yesterday https://bugs.ruby-lang.org/issues/4475. It looks like [1, 2, 3].map { @1 * 2 }. I'm pretty excited about it! But yeah we don't support it so the test is failing. Honestly HEAD should probably be an allowed failure.

@kddnewton
Copy link
Member

Okay I added support for that node type, so if you wouldn't mind rebasing it should work again.

@johnschoeman
Copy link
Contributor Author

And rebased. @kddeisz

johnschoeman and others added 2 commits March 29, 2019 13:35
Why:
We would like to be able to have explicit tests that will allow to
ensure that prettier-ruby behaves as intended. Ideally these tests can
be run independently and could allow for TDD.

This commit:
introduces an assert_equal_and_valid method which takes an input string
and an expected output string and will validate that prettier can be
applied to the expected string by:

writing the expected string to a Tempfile, 'prettier-ruby-test.rb',
applying prettier-ruby to the Tempfile with the 'bin/print' executable,
asserting with minitest that the resultant file is equal to the expected
string,
and executing the resultant file with Open3 to capture any stderr and
status to validate that the formatted code can be run without throwing
an error.

Two example specs are provided in this commit to demonstrate the spec
helpers usage

Co-authored-by: christoomey <chris@thoughtbot.com>
Co-authored-by: johnschoeman <johnschoeman1617@gmail.com>
Why:
Currently if a block is given just a parameter and no actual content the
formatter will return a block with an extra space, which is a rubocop
error: 'Layout/ExtraSpacing: Layout/ExtraSpacing: Unnecessary spacing
detected.'

for example: { |el| } -> { |el|  }

This commit:
Updates the block node to not add an extra space if the block has no
contents.

Co-authored-by: christoomey <chris@thoughtbot.com>
Co-authored-by: johnschoeman <johnschoeman1617@gmail.com>
@kddnewton kddnewton mentioned this pull request Apr 13, 2019
@kddnewton
Copy link
Member

Thank you thank you thank you for your work on this. Even though this isn't the direction I ended up going in for the tests, I really appreciate the contribution.

@kddnewton kddnewton closed this Apr 17, 2019
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.

2 participants