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

More pretty print tests #232

Closed
wants to merge 2 commits into from

Conversation

@AydinHassan
Copy link
Contributor

commented Oct 1, 2015

Hey, I was using the PrinterPrinter for something and noticed that a lot of the code was not tested, so I decided to add some tests. See 818cc6.

I also added a multi init for loop parser test. See 2188234.


}

for ($i = 0; $ < 10;) {

This comment has been minimized.

Copy link
@nikic

nikic Oct 1, 2015

Owner

The $ here is missing a variable name -- that's probably what's causing the issue.

The error message in the test is really bad though, that should be done better...

This comment has been minimized.

Copy link
@AydinHassan

AydinHassan Oct 1, 2015

Author Contributor

Wow, that's embarrassing :(

@AydinHassan AydinHassan force-pushed the AydinHassan:more-pretty-print-tests branch from 21dfa54 to 2188234 Oct 1, 2015

@AydinHassan

This comment has been minimized.

Copy link
Contributor Author

commented Oct 1, 2015

@nikic Fixed and updated the PR description, now excuse me while I go back to page 1 in my PHP book 😭

@nikic

This comment has been minimized.

Copy link
Owner

commented Oct 2, 2015

Thanks for the tests! That brings pretty printer coverage to 91% :)

@nikic nikic closed this Oct 2, 2015

@AydinHassan

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2015

Thanks 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.