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

Parser: Pass-through PHP errors in tests #11429

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Nov 2, 2018

In #11320 we started running all parsers against the same set of unit
tests. Unfortunately when the PHP-based parsers failed inside the PHP
process or returned non-JSON data then the tests would fail without
providing any information about why.

My personal workaround was to manually run the PHP test runner from
the command line to see the output. This was inefficient.

In this patch we're trapping the response code from the PHP test runner
and throwing an Error with the output from php so that our jest
tests can see and report them. This will make it easier debug failing
tests.

Testing

You can see the impact of the change by introducing a syntax error in the
default PHP parser. Inside the parse() method I added a new line with
<!-- as its contents - invalid PHP.

In master I ran: npm run test-unit packages/block-serializer-default-parser

 ● block-serialization-default-parser-php › generic tests › should grab HTML soup after blocks

    SyntaxError: Unexpected token P in JSON at position 1
        at JSON.parse (<anonymous>)

In this branch though…

  ● block-serialization-default-parser-php › generic tests › should grab HTML soup after blocks


    Parse error: syntax error, unexpected '<' in /Users/dmsnell/PhpstormProjects/gutenberg/packages/block-serialization-default-parser/parser.php on line 183

In #11320 we started running all parsers against the same set of unit
tests. Unfortunately when the PHP-based parsers failed inside the PHP
process or returned non-JSON data then the tests would fail without
providing any information about why.

My personal workaround was to manually run the PHP test runner from
the command line to see the output. This was inefficient.

In this patch we're trapping the response code from the PHP test runner
and throwing an `Error` with the output from `php` so that our `jest`
tests can see and report them. This will make it easier debug failing
tests.
@dmsnell dmsnell added the [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f label Nov 2, 2018
@dmsnell dmsnell requested review from mcsf, pento, gziolo and aduth November 2, 2018 17:28
@dmsnell dmsnell merged commit 2479293 into master Nov 2, 2018
@dmsnell dmsnell deleted the parser/pass-through-php-errors-in-tests branch November 2, 2018 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants