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

After 2.3.2->2.4.0 update, Behat stops on the first feature involving an exception #1028

Closed
guilliamxavier opened this issue Jun 22, 2020 · 6 comments · Fixed by #1030
Closed
Milestone

Comments

@guilliamxavier
Copy link
Contributor

Refs Behat/Behat#1302

After updating sentry/sentry to 2.4.0 (from 2.3.2), when Behat encounters a feature that [contains a scenario that] involves an exception (for example

    When I send a "GET" request to "/dummy"
    Then the response status code should be 404

with Symfony), then it stops after this file, not running the remaining features in the test suite.

I've tracked the regression down to commit 498a41c (PR #991).

After some debugging, it appears that the problem occurs when $value is an instance of
\Behat\Behat\Gherkin\Specification\LazyFeatureIterator and/or
\Behat\Testwork\Specification\GroupedSpecificationIterator
(I guess the serialization exhausts the current suite iterator).

@Jean85
Copy link
Contributor

Jean85 commented Jun 23, 2020

You can probably work around the issue by defining a custom class serializer for those classes, return a simple string and avoid consuming the iterator.

@guilliamxavier
Copy link
Contributor Author

@Jean85: Thanks but unfortunately that won't work: the current code does (comments mine):

            /* ... */

            if (is_iterable($value)) { // formerly is_array($value)
                $serializedArray = [];

                foreach ($value as $k => $v) { // <-- consumes the iterator
                    $serializedArray[$k] = $this->serializeRecursively($v, $_depth + 1);
                }

                return $serializedArray;
            }

            if (\is_object($value)) { // Iterator doesn't get to here anymore
                $classSerializers = $this->resolveClassSerializers($value);

                /* ... */
            }

            /* ... */

So I don't see how to work around... And since #991 was

What about using is_iterable to allow Traversable objects to be processed automatically?

(i.e. just a suggestion), and

If the check is only to be able to do a foreach, I think it's safe.

(which turned out to not be), then maybe it should simply be reverted (and add a non-regression test)?

@ste93cry
Copy link
Contributor

then maybe it should simply be reverted

I agree, we clearly didn't think about all implications of serializing an iterable. Would you mind opening a PR to revert the change?

@guilliamxavier
Copy link
Contributor Author

@ste93cry: TBH I probably wouldn't have thought either... I have opened a PR

@Jean85
Copy link
Contributor

Jean85 commented Jun 24, 2020

Also we may encounter non-rewindable iterators, which is another bad edge case for this feature...

@guilliamxavier
Copy link
Contributor Author

@Jean85: Yes, the linked PR tests a simple ArrayIterator but also a Generator which is both non-rewindable and non-cloneable

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 a pull request may close this issue.

4 participants