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

assert not working in anonymous class #5651

Closed
root-aza opened this issue Jan 10, 2024 · 12 comments
Closed

assert not working in anonymous class #5651

root-aza opened this issue Jan 10, 2024 · 12 comments
Labels
type/bug Something is broken version/10 Something affects PHPUnit 10

Comments

@root-aza
Copy link

Q A
PHPUnit version 10.5.5
PHP version 8.2.13
Installation Method Composer / PHAR

Summary

Current behavior

assert do not fire, although the condition is definitely false

How to reproduce

Repository with test

Expected behavior

@root-aza root-aza added type/bug Something is broken version/10 Something affects PHPUnit 10 labels Jan 10, 2024
@marac19901990
Copy link
Contributor

@root-aza I tried to reproduce the issue and i don't see any issues. My guess is that you didn't call the function inside the anonyomous class thus the assert didn't get triggered.

@root-aza
Copy link
Author

@marac19901990 mmm. No))

Screenshot 2024-01-11 at 09 26 00
Screenshot 2024-01-11 at 09 25 46

Screenshot 2024-01-11 at 09 26 37
Screenshot 2024-01-11 at 09 26 34

@marac19901990
Copy link
Contributor

The problem is that i can'tt reproduce it in a minimal setup, which would allow me to ads it to the tests in phpunit. In your repository example there are a lot of dependencies which are not relevant for this project, will try some more...

@marac19901990
Copy link
Contributor


namespace PHPUnit\Framework;

class AnonymousClassTest extends TestCase
{
    public function testAssertInAnonymousClass()
    {
        $a = new Class implements SomeClass {
            public function test(string $a): string
            {
                assertEquals(is_string($a), true);
                return $a;
            }
        };

        $a->test('someString');
    }
}

interface SomeClass {
    public function test(string $a): string;
}```

This minimal test works as expected.

@root-aza
Copy link
Author

Well, in a minimal setup I think it works for everyone =))
We need to consider specific cases...

And it seems to me that it doesn’t matter what code is being tested, the framework should report an assert. Maybe I'm wrong...

@root-aza
Copy link
Author

root-aza commented Jan 11, 2024

And it seems to me that it doesn’t matter what code is being tested, the framework should report an assert. Maybe I'm wrong...

It is clear that everywhere has its limitations, but it seems to me that the framework should freely handle this case.

@marac19901990
Copy link
Contributor

marac19901990 commented Jan 11, 2024

@root-aza
I debugged the code from your Repository:

Instead of $client = $builder->getClient();
The right client is $client = $builder->getHttpClient();
With this changr the assertion works as expected.

@sebastianbergmann you can close this

@root-aza
Copy link
Author

root-aza commented Jan 11, 2024

@root-aza I debugged the code from your Repository:

Instead of $client = $builder->getClient(); The right client is $client = $builder->getHttpClient(); With this changr the assertion works as expected.

@sebastianbergmann you can close this

Um. this is not correct and the code will fail in typing.

1) Vanta\Integration\Temporal\Sentry\Test\SentryActivityInboundInterceptorTest::testHandleThrowable
Failed asserting that exception of type "TypeError" matches expected exception "RuntimeException". Message was: "Sentry\State\Hub::bindClient(): Argument #1 ($client) must be of type Sentry\ClientInterface, Sentry\HttpClient\HttpClientInterface@anonymous given, called in /Users/aza/PhpstormProjects/temporal-sentry/tests/SentryActivityInboundInterceptorTest.php on line 138" at

UPD:
these are different contracts

Screenshot 2024-01-11 at 10 26 50

@marac19901990
Copy link
Contributor

@root-aza I debugged the code from your Repository:
Instead of $client = $builder->getClient(); The right client is $client = $builder->getHttpClient(); With this changr the assertion works as expected.
@sebastianbergmann you can close this

Um. this is not correct and the code will fail in typing.

1) Vanta\Integration\Temporal\Sentry\Test\SentryActivityInboundInterceptorTest::testHandleThrowable
Failed asserting that exception of type "TypeError" matches expected exception "RuntimeException". Message was: "Sentry\State\Hub::bindClient(): Argument #1 ($client) must be of type Sentry\ClientInterface, Sentry\HttpClient\HttpClientInterface@anonymous given, called in /Users/aza/PhpstormProjects/temporal-sentry/tests/SentryActivityInboundInterceptorTest.php on line 138" at

UPD: these are different contracts

Screenshot 2024-01-11 at 10 26 50

I didn't explain myself well, sorry. I just wanted to say that when i do this:

        $throwable = new RuntimeException('Oops!');

//        $this->expectExceptionObject($throwable);

        $builder = ClientBuilder::create([
            'http_client' => new class() implements HttpClient {
                public function sendRequest(Request $request, Options $options): Response
                {
                    $mainRequest = [];

//                    assertArrayHasKey('contexts', $mainRequest);
//                    assertArrayHasKey('Workflow', $mainRequest['contexts']);
//
//                    assertArrayHasKey('Id', $mainRequest['contexts']['Workflow']);
//                    assertEquals('f06e87b1-5e56-4c5d-a789-3f68a7a3af14', $mainRequest['contexts']['Workflow']['Id']);
//
//                    assertArrayHasKey('Type', $mainRequest['contexts']['Workflow']);
//                    assertEquals('Test', $mainRequest['contexts']['Workflow']['Type']);
//
//                    assertArrayHasKey('Activity', $mainRequest['contexts']);
//
//                    assertArrayHasKey('Id', $mainRequest['contexts']['Activity']);
//                    assertEquals('92dbc19f-2206-4229-85b7-2ca5cb6ada4a', $mainRequest['contexts']['Activity']['Id']);
//
//                    assertArrayHasKey('Type', $mainRequest['contexts']['Activity']);
//                    assertEquals('Test', $mainRequest['contexts']['Activity']['Type']);
//
//                    assertArrayHasKey('TaskQueue', $mainRequest['contexts']['Activity']);

                    assertEquals('asdasd', 'not matching string');
                    return new Response(200, [], '');
                }
            },
            'dsn' => 'https://[email protected]/52',
        ]);

        $client = $builder->getHttpClient();
        $client->sendRequest(new Request(), new Options());
        exit;

The assert gets executed, so it is not a phpunit issue, I didn't go further into debugging what happens.
Hope I helped somehow :)

@marac19901990
Copy link
Contributor

marac19901990 commented Jan 11, 2024

@root-aza
HttpTransport:

        } catch (\Throwable $exception) {
            $this->logger->error(
                sprintf('Failed to send the event to Sentry. Reason: "%s".', $exception->getMessage()),
                ['exception' => $exception, 'event' => $event]
            );
            // If you add this the assertions will run
            throw $exception;
            return new Result(ResultStatus::failed());
        }

if you rethrow the exception it will run the assertions, this shows that the sentry package captures the exceptions and doesnt propagate them further, and that is why phpunit doesn't register them.
I would test the HttpRequest class separately which would eliminate this problem.
This should clear your doubts I think :)

@root-aza
Copy link
Author

root-aza commented Jan 11, 2024

if you rethrow the exception it will run the assertions, this shows that the sentry package captures the exceptions and doesnt propagate them further, and that is why phpunit doesn't register them.
I would test the HttpRequest class separately which would eliminate this problem.
This should clear your doubts I think :)

Yep.
But it seems to me that phpunit should be able to beat these cases. Do not fire exceptions in assert, but, for example, throw failures through fibers.

@sebastianbergmann
Copy link
Owner

I have no idea what you are trying to report here, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken version/10 Something affects PHPUnit 10
Projects
None yet
Development

No branches or pull requests

3 participants