Page MenuHomePhabricator

PHPUnit: Cover all scenarios for WMFBaseDomainExtractor class
Closed, ResolvedPublic

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%

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

D3r1ck01 renamed this task from PHPUnit: Cover all scenario for WMFBaseDomainExtractor class to PHPUnit: Cover all scenarios for WMFBaseDomainExtractor class.Nov 10 2018, 6:21 PM
D3r1ck01 claimed this task.
D3r1ck01 triaged this task as Normal priority.
D3r1ck01 moved this task from Backlog to Doing [WIP] on the User-D3r1ck01 board.
D3r1ck01 added a comment.EditedNov 10 2018, 7:12 PM

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
D3r1ck01 updated the task description. (Show Details)EditedNov 10 2018, 7:53 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.

Now CC gets to 100% which matches what the CC I got locally generated: https://integration.wikimedia.org/ci/job/mwext-phpunit-coverage-patch-docker/5550/console.

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

Change 472795 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove unreachable code and add more test cases + docs

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

D3r1ck01 updated the task description. (Show Details)Nov 22 2018, 7:24 PM
D3r1ck01 closed this task as Resolved.Nov 22 2018, 8:01 PM
D3r1ck01 updated the task description. (Show Details)

Thanks @pmiazga & @Jdlrobson. Resolving this :}