Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Tests] Streamline#52365

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:5.4fromOskarStark:fix/tests-4
Oct 31, 2023
Merged

Conversation

@OskarStark
Copy link
Contributor

@OskarStarkOskarStark commentedOct 30, 2023
edited
Loading

QA
Branch?5.4
Bug fix?no
New feature?no
Deprecations?no
Issues--
LicenseMIT

Extracted from#52157

Comment on lines +35 to 53
publicfunctiontestNoTwigTemplateActionMethod()
{
$controller =newTemplateController();

$this->expectException(\LogicException::class);
$this->expectExceptionMessage('You cannot use the TemplateController if the Twig Bundle is not available.');
$controller =newTemplateController();

$controller->templateAction('mytemplate')->getContent();
}

publicfunctiontestNoTwigInvokeMethod()
{
$controller =newTemplateController();

$this->expectException(\LogicException::class);
$this->expectExceptionMessage('You cannot use the TemplateController if the Twig Bundle is not available.');

$controller('mytemplate')->getContent();
}
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

We should test both ways independently


$client =$this->createClient(['test_case' =>'StandardFormLogin','root_config' =>'invalid_ip_access_control.yml']);
$client->request('GET','/unprotected_resource');
$this->createClient(['test_case' =>'StandardFormLogin','root_config' =>'invalid_ip_access_control.yml']);
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Exception is thrown while creating the Client, no need to callrequest()


$controller =newProfilerController($urlGenerator,null,$twig, []);
$controller->phpinfoAction(Request::create('/_profiler/phpinfo'));
$controller->phpinfoAction();
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

phpInfoAction has no parameter

@OskarStark
Copy link
ContributorAuthor

Ready to merge from my side

@OskarStark
Copy link
ContributorAuthor

@nicolas-grekas as PRs get bigger and bigger, can you please merge/upmerge this one? Thanks

Comment on lines -1022 to +1020
$table =newTable($output =$this->getOutputStream());
$table =newTable($this->getOutputStream());
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Unused variable$output

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('A cell must be a TableCell, a scalar or an object implementing "__toString()", "array" given.');

$table->render();
Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@stof it seems, that this is no dead code, as the exception is raised when callingrender() and not when callingsetRows() 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

then in the other PR, keepsetRows in initialization

@OskarStarkOskarStark requested a review fromstofOctober 31, 2023 07:27
@OskarStark
Copy link
ContributorAuthor

Fabbot looks unrelated

@nicolas-grekas
Copy link
Member

Thank you@OskarStark.

OskarStark and sstok reacted with heart emoji

@nicolas-grekasnicolas-grekas merged commitb26fdd0 intosymfony:5.4Oct 31, 2023
@OskarStarkOskarStark deleted the fix/tests-4 branchOctober 31, 2023 08:06
nicolas-grekas added a commit that referenced this pull requestOct 31, 2023
…e expectation to avoid false positives (OskarStark)This PR was merged into the 6.4 branch.Discussion----------[Tests] Move `expectException` closer to the place of the expectation to avoid false positives| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | --| License       | MITIf you guys like this PR, I will fix the other places as wellShould be merged after merge & upmerge of:*#52365Commits-------6115ab0 [Tests] Move expectException closer to the place of the expectation to avoid false positives
@OskarStarkOskarStark mentioned this pull requestNov 1, 2023
nicolas-grekas added a commit that referenced this pull requestDec 26, 2023
This PR was squashed before being merged into the 7.1 branch.Discussion----------[Tests] Streamline| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | --| License       | MITFollows*#52157*#52365Commits-------33d71aa [Tests] Streamline
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ycerutoycerutoyceruto approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@stofstofAwaiting requested review from stof

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

5 participants

@OskarStark@nicolas-grekas@stof@yceruto@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp