Skip to content

Conversation

akomm
Copy link
Contributor

@akomm akomm commented Mar 28, 2019

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets #...
License MIT

I had clean clone & checkout test failure on \Overblog\GraphQLBundle\Tests\Functional\Upload\UploadTest::testParseLiteralIsUnsupported.

There are a few problems I have detected:

  1. The VersionHelper::webonyxGraphQLPHPVersion lookup for composer is prone. It uses which composer as fallback. But composer might also be available as an alias.
  2. The Process success is tested, but the command has multiple processes executed (pipe to grep) and so the return code of the last is tested against, which is grep. It caused a false positive, even though which composer resolved to empty string and the execution failed with unknown command show.

Result:
When failing, you get no feedback what the actually issue is: fail to resolve the composer and/or version of webonyx/graphql-php..

So I wanted to fix this in a way which works with alias and gives proper feedback on failure, but realized it gets really messy. On top of this I also realized, that this actually tests vendor behavior, which a test should not do. So my PR is an opinionated proposal to not test explicit vendor behavior but for the possible outcome.

@akomm akomm requested a review from mcg-web March 29, 2019 14:34
@mcg-web
Copy link
Contributor

mcg-web commented Mar 29, 2019

Thank you @akomm 👍

@mcg-web mcg-web merged commit 0cc8659 into overblog:master Mar 29, 2019
@akomm akomm deleted the fix-upload-test branch August 2, 2019 10:42
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