NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Use triggering file to determine weak vendors if when the test is run in a separate process#25685

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

Jump to bottom

Closed

alexpott wants to merge2 commits intosymfony:3.4fromalexpott:weak_vendors_process_isolation

Closed

Use triggering file to determine weak vendors if when the test is run in a separate process#25685

alexpott wants to merge2 commits intosymfony:3.4fromalexpott:weak_vendors_process_isolation

Conversation

Copy link

Contributor

alexpott commentedJan 4, 2018•
edited by nicolas-grekas
Loading

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	I think so
Fixed tickets	#25684
License	MIT
Doc PR	symfony/symfony-docs#...

carsonbot added Status: Needs Review Bug labels

Jan 4, 2018

nicolas-grekas changed the title~~Use triggering file to determine weak vendors if when the test is run…~~Use triggering file to determine weak vendors if when the test is run in a separate process

Jan 4, 2018

nicolas-grekas changed the base branch frommaster to3.4

January 4, 2018 16:22

nicolas-grekas added this to the3.4 milestone

Jan 4, 2018

Copy link

Member

nicolas-grekas commentedJan 4, 2018

can you rebase on 3.4 please ?

Use triggering file to determine weak vendors if when the test is run…

14925cb

… in a separate process

alexpott force-pushed theweak_vendors_process_isolation branch from4f05ef4 to14925cbCompare

January 4, 2018 16:26

Copy link

ContributorAuthor

alexpott commentedJan 4, 2018

Rebased on 3.4. I can't work out how to write a test for this because atm all the weak vendor stuff is tested via phpt tests and a fake vendor.

nicolas-grekas reviewed

Jan 4, 2018

View reviewed changes

src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

		$msg =$parsedMsg['deprecation'];
		$class =$parsedMsg['class'];
		$method =$parsedMsg['method'];
		$isWeak = DeprecationErrorHandler::MODE_WEAK ===$mode \|\| (DeprecationErrorHandler::MODE_WEAK_VENDORS ===$mode &&$isVendor =$inVendors($parsedMsg['triggering_file']));

Copy link

Member

nicolas-grekasJan 4, 2018

Choose a reason for hiding this comment

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

shouldn't $isVendor be initialized to false before this line?

Copy link

Member

nicolas-grekasJan 4, 2018

Choose a reason for hiding this comment

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

then L124:$isWeak = $isWeak || ...?

Copy link

Member

nicolas-grekasJan 4, 2018•
edited
Loading

Choose a reason for hiding this comment

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

self answering :)

shouldn't $isVendor be initialized to false before this line?

not needed as logically this wouldn't make a difference, but might still help the reader

then L124: $isWeak = $isWeak || ...?

really not sure about that (might instead break the fix, you'll tell :) )

Copy link

ContributorAuthor

alexpottJan 4, 2018

Choose a reason for hiding this comment

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

Yeah that'd break the fix. The point is $isWeak is wrong at this point. Can always add a comment.

Add a comment

bdcd52f

nicolas-grekas approved these changes

Jan 4, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jan 4, 2018

Copy link

Member

nicolas-grekas commentedJan 4, 2018

Thank you@alexpott.

nicolas-grekas closed this in228453b

Jan 4, 2018

This was referencedJan 5, 2018

Release v3.4.3#25691

Merged

Release v4.0.3#25692

Merged

alexpott mentioned this pull request

Jan 5, 2018

[PhpUnitBridge]allow outdated vendors mode#24867

Closed

Copy link

Contributor

greg0ire commentedJan 5, 2018

Oh wow you fixed it, thanks! Could have pinged me though :P

greg0ire reviewed

Jan 5, 2018

View reviewed changes

src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

		// then we need to use the serialized information to
		// determine if the error has been triggered from vendor
		// code.
		$isWeak = DeprecationErrorHandler::MODE_WEAK ===$mode \|\| (DeprecationErrorHandler::MODE_WEAK_VENDORS ===$mode &&$isVendor =$inVendors($parsedMsg['triggering_file']));