Maniphest T209218

PHPUnit: Cover all scenarios for WMFBaseDomainExtractor class
Closed, ResolvedPublic
Actions

Description

In this class, not all scenarios are tested and coverage is currently not 100%. In an attempt to solve this issue, it's noticed that this is due to code that is never executed.

The below code snippets are never executed;

 if ( count( $subdomains ) == 0 ) {
            return $baseDomain;  // --> this line (check is executed but never returns 0)
}

So, this code can be safely removed? I've done end to end testing on this and yes, we can remove this code.

Acceptance Criteria

Remove unreachable / untestable code
Coverage should go up to 96.00%

Related Objects
Search...

		Status	Assigned	Task
		Open	None	T208761 [Epic] Improve PHPUnit coverage in MobileFrontend to medium
		Resolved	D3r1ck01	T209218 PHPUnit: Cover all scenarios for WMFBaseDomainExtractor class

Event Timeline

D3r1ck01 created this task.Nov 10 2018, 6:20 PM

Change 472795 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/MobileFrontend@master] Add array of empty strings to getBaseDomainProvider()

https://gerrit.wikimedia.org/r/472795

gerritbot added a project: Patch-For-Review.Nov 10 2018, 6:20 PM

D3r1ck01 moved this task from Backlog to Doing [WIP] on the User-D3r1ck01 board.Nov 10 2018, 6:21 PM

D3r1ck01 claimed this task.

D3r1ck01 triaged this task as Normal priority.

D3r1ck01 renamed this task from PHPUnit: Cover all scenario for WMFBaseDomainExtractor class to PHPUnit: Cover all scenarios for WMFBaseDomainExtractor class.

Having this error;

20:03:44 Generating code coverage report in Clover XML format ... done
20:03:44 PHP Warning:  file_get_contents(WMFBaseDomainExtractor.php): failed to open stream: No such file or directory in /opt/phpunit-patch-coverage/vendor/mediawiki/phpunit-patch-coverage/src/CheckCommand.php on line 127
20:03:44 $ php7.0 -d zend_extension=xdebug.so "$MW_INSTALL_PATH"/tests/phpunit/phpunit.php --coverage-clover /tmp/cloverjfMZP0 --filter '/WMFBaseDomainExtractorTest/'

Don't know why the report can't be generated for me to know if it's increased and how many percent etc.

See https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch-docker/5549/console

D3r1ck01 updated the task description. (Show Details)Nov 10 2018, 7:50 PM

So after a while of digging, I thought it was the test case that doesn't cover the scenarios the code needs to execute but rather the code doesn't execute in any of the scenarios and will never do.

Based on $wmfWikiHosts and $wmfMultiDomainWikiHosts and the implementation of getCookieDomain(), the snippets in the description will never be executed on the data provider given and no test cases executes this code.