Skip to content

[Tests] Move expectException closer to the place of the expectation to avoid false positives #52157

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

Merged
merged 1 commit into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class RegisterEventListenersAndSubscribersPassTest extends TestCase

public function testExceptionOnAbstractTaggedSubscriber()
{
$this->expectException(\InvalidArgumentException::class);
$container = $this->createBuilder();

$abstractDefinition = new Definition('stdClass');
Expand All @@ -36,12 +35,13 @@ public function testExceptionOnAbstractTaggedSubscriber()

$container->setDefinition('a', $abstractDefinition);

$this->expectException(\InvalidArgumentException::class);

$this->process($container);
}

public function testExceptionOnAbstractTaggedListener()
{
$this->expectException(\InvalidArgumentException::class);
$container = $this->createBuilder();

$abstractDefinition = new Definition('stdClass');
Expand All @@ -50,6 +50,8 @@ public function testExceptionOnAbstractTaggedListener()

$container->setDefinition('a', $abstractDefinition);

$this->expectException(\InvalidArgumentException::class);

$this->process($container);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ class RegisterMappingsPassTest extends TestCase
{
public function testNoDriverParmeterException()
{
$container = $this->createBuilder();

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Could not find the manager name parameter in the container. Tried the following parameter names: "manager.param.one", "manager.param.two"');
$container = $this->createBuilder();

$this->process($container, [
'manager.param.one',
'manager.param.two',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ protected function setUp(): void

public function testFixManagersAutoMappingsWithTwoAutomappings()
{
$this->expectException(\LogicException::class);
$emConfigs = [
'em1' => [
'auto_mapping' => true,
Expand All @@ -76,6 +75,8 @@ public function testFixManagersAutoMappingsWithTwoAutomappings()
$reflection = new \ReflectionClass($this->extension);
$method = $reflection->getMethod('fixManagersAutoMappings');

$this->expectException(\LogicException::class);

$method->invoke($this->extension, $emConfigs, $bundles);
}

Expand Down Expand Up @@ -255,8 +256,6 @@ public function testServiceCacheDriver()

public function testUnrecognizedCacheDriverException()
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('"unrecognized_type" is an unrecognized Doctrine cache driver.');
$cacheName = 'metadata_cache';
$container = $this->createContainer();
$objectManager = [
Expand All @@ -266,6 +265,9 @@ public function testUnrecognizedCacheDriverException()
],
];

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('"unrecognized_type" is an unrecognized Doctrine cache driver.');

$this->invokeLoadCacheDriver($objectManager, $container, $cacheName);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,6 @@ public function testLoadValuesForChoicesDoesNotLoadIfEmptyChoices()

public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Not defining the IdReader explicitly as a value callback when the query can be optimized is not supported.');

$loader = new DoctrineChoiceLoader(
$this->om,
$this->class,
Expand All @@ -169,7 +166,10 @@ public function testLoadValuesForChoicesDoesNotLoadIfSingleIntId()
->with($this->obj2)
->willReturn('2');

$this->assertSame(['2'], $loader->loadValuesForChoices([$this->obj2]));
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Not defining the IdReader explicitly as a value callback when the query can be optimized is not supported.');

$loader->loadValuesForChoices([$this->obj2]);
}

public function testLoadValuesForChoicesDoesNotLoadIfSingleIntIdAndValueGiven()
Expand Down Expand Up @@ -253,18 +253,13 @@ public function testLoadChoicesForValuesDoesNotLoadIfEmptyValues()

public function testLegacyLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
{
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Not defining the IdReader explicitly as a value callback when the query can be optimized is not supported.');

$loader = new DoctrineChoiceLoader(
$this->om,
$this->class,
$this->idReader,
$this->objectLoader
);

$choices = [$this->obj2, $this->obj3];

$this->idReader->expects($this->any())
->method('getIdField')
->willReturn('idField');
Expand All @@ -275,10 +270,10 @@ public function testLegacyLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader
$this->objectLoader->expects($this->never())
->method('getEntitiesByIds');

$this->assertSame(
[4 => $this->obj3, 7 => $this->obj2],
$loader->loadChoicesForValues([4 => '3', 7 => '2'])
);
$this->expectException(LogicException::class);
$this->expectExceptionMessage('Not defining the IdReader explicitly as a value callback when the query can be optimized is not supported.');

$loader->loadChoicesForValues([4 => '3', 7 => '2']);
}

public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueUseIdReader()
Expand Down Expand Up @@ -374,15 +369,15 @@ public function testLoadChoicesForValuesLoadsOnlyChoicesIfValueIsIdReader()

public function testPassingIdReaderWithoutSingleIdEntity()
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('The "$idReader" argument of "Symfony\\Bridge\\Doctrine\\Form\\ChoiceList\\DoctrineChoiceLoader::__construct" must be null when the query cannot be optimized because of composite id fields.');

$idReader = $this->createMock(IdReader::class);
$idReader->expects($this->once())
->method('isSingleId')
->willReturn(false)
;

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('The "$idReader" argument of "Symfony\\Bridge\\Doctrine\\Form\\ChoiceList\\DoctrineChoiceLoader::__construct" must be null when the query cannot be optimized because of composite id fields.');

new DoctrineChoiceLoader($this->om, $this->class, $idReader, $this->objectLoader);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public function testConfigureQueryBuilderWithNonQueryBuilderAndNonClosure()
public function testConfigureQueryBuilderWithClosureReturningNonQueryBuilder()
{
$this->expectException(UnexpectedTypeException::class);
$field = $this->factory->createNamed('name', static::TESTED_TYPE, null, [
$this->factory->createNamed('name', static::TESTED_TYPE, null, [
'em' => 'default',
'class' => self::SINGLE_IDENT_CLASS,
'query_builder' => fn () => new \stdClass(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,16 @@ public function testMiddlewareWrapsInTransactionAndFlushes()

public function testTransactionIsRolledBackOnException()
{
$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Thrown from next middleware.');
$this->connection->expects($this->once())
->method('beginTransaction')
;
$this->connection->expects($this->once())
->method('rollBack')
;

$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Thrown from next middleware.');

$this->middleware->handle(new Envelope(new \stdClass()), $this->getThrowingStackMock());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ public function testLoadUserByIdentifierWithUserLoaderRepositoryAndWithoutProper

public function testLoadUserByIdentifierWithNonUserLoaderRepositoryAndWithoutProperty()
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('You must either make the "Symfony\Bridge\Doctrine\Tests\Fixtures\User" entity Doctrine Repository ("Doctrine\ORM\EntityRepository") implement "Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface" or set the "property" option in the corresponding entity provider configuration.');
$em = DoctrineTestHelper::createTestEntityManager();
$this->createSchema($em);

Expand All @@ -100,6 +98,10 @@ public function testLoadUserByIdentifierWithNonUserLoaderRepositoryAndWithoutPro
$em->flush();

$provider = new EntityUserProvider($this->getManager($em), 'Symfony\Bridge\Doctrine\Tests\Fixtures\User');

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('You must either make the "Symfony\Bridge\Doctrine\Tests\Fixtures\User" entity Doctrine Repository ("Doctrine\ORM\EntityRepository") implement "Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface" or set the "property" option in the corresponding entity provider configuration.');

$provider->loadUserByIdentifier('user1');
}

Expand Down Expand Up @@ -171,14 +173,15 @@ public function testLoadUserByIdentifierShouldLoadUserWhenProperInterfaceProvide

public function testLoadUserByIdentifierShouldDeclineInvalidInterface()
{
$this->expectException(\InvalidArgumentException::class);
$repository = $this->createMock(ObjectRepository::class);

$provider = new EntityUserProvider(
$this->getManager($this->getObjectManager($repository)),
'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
);

$this->expectException(\InvalidArgumentException::class);

$provider->loadUserByIdentifier('name');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,6 @@ public function testValidateUniquenessWithArrayValue()

public function testDedicatedEntityManagerNullObject()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('Object manager "foo" does not exist.');
$constraint = new UniqueEntity([
'message' => 'myMessage',
'fields' => ['name'],
Expand All @@ -632,13 +630,14 @@ public function testDedicatedEntityManagerNullObject()

$entity = new SingleIntIdEntity(1, null);

$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('Object manager "foo" does not exist.');

$this->validator->validate($entity, $constraint);
}

public function testEntityManagerNullObject()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('Unable to find the object manager associated with an entity of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity"');
$constraint = new UniqueEntity([
'message' => 'myMessage',
'fields' => ['name'],
Expand All @@ -652,6 +651,9 @@ public function testEntityManagerNullObject()

$entity = new SingleIntIdEntity(1, null);

$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('Unable to find the object manager associated with an entity of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity"');

$this->validator->validate($entity, $constraint);
}

Expand Down Expand Up @@ -719,8 +721,6 @@ public function testValidateInheritanceUniqueness()

public function testInvalidateRepositoryForInheritance()
{
$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('The "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity" entity repository does not support the "Symfony\Bridge\Doctrine\Tests\Fixtures\Person" entity. The entity should be an instance of or extend "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity".');
$constraint = new UniqueEntity([
'message' => 'myMessage',
'fields' => ['name'],
Expand All @@ -729,6 +729,10 @@ public function testInvalidateRepositoryForInheritance()
]);

$entity = new Person(1, 'Foo');

$this->expectException(ConstraintDefinitionException::class);
$this->expectExceptionMessage('The "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity" entity repository does not support the "Symfony\Bridge\Doctrine\Tests\Fixtures\Person" entity. The entity should be an instance of or extend "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity".');

$this->validator->validate($entity, $constraint);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,14 @@ public function testCreateUploadedFile()

public function testCreateUploadedFileWithError()
{
$this->expectException(FileException::class);
$this->expectExceptionMessage('The file "e" could not be written on disk.');

$uploadedFile = $this->createUploadedFile('Error.', \UPLOAD_ERR_CANT_WRITE, 'e', 'text/plain');
$symfonyUploadedFile = $this->callCreateUploadedFile($uploadedFile);

$this->assertEquals(\UPLOAD_ERR_CANT_WRITE, $symfonyUploadedFile->getError());

$this->expectException(FileException::class);
$this->expectExceptionMessage('The file "e" could not be written on disk.');

$symfonyUploadedFile->move($this->tmpDir, 'shouldFail.txt');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ class HttpKernelExtensionTest extends TestCase
{
public function testFragmentWithError()
{
$this->expectException(\Twig\Error\RuntimeError::class);
$renderer = $this->getFragmentHandler($this->throwException(new \Exception('foo')));

$this->expectException(\Twig\Error\RuntimeError::class);

$this->renderTemplate($renderer);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,16 @@ public function testDebugCustomDirectory()

public function testDebugInvalidDirectory()
{
$this->expectException(\InvalidArgumentException::class);
$kernel = $this->createMock(KernelInterface::class);
$kernel->expects($this->once())
->method('getBundle')
->with($this->equalTo('dir'))
->willThrowException(new \InvalidArgumentException());

$tester = $this->createCommandTester([], [], $kernel);

$this->expectException(\InvalidArgumentException::class);

$tester->execute(['locale' => 'en', 'bundle' => 'dir']);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@ public function testLintIncorrectFile()

public function testLintFileNotReadable()
{
$this->expectException(\RuntimeException::class);
$tester = $this->createCommandTester();
$filename = $this->createFile('');
unlink($filename);

$this->expectException(\RuntimeException::class);

$tester->execute(['filename' => $filename], ['decorated' => false]);
}

Expand Down
Loading