Problem/Motivation

Similar to #2951843: Use drupalci.yml to fail test runs early

Before too long, we'll have a way to specify a build process for core, independent of the testbot: #2949209: Phase I: Allow drupalci.yml to influence the build

This issue might be a place to sort that out, but for now, until it's finalized, we also need a place to put a patch that adds a build file to a contrib module. That way we can work on the testbot: #2950939: Use the drupalci.yml to specify assessment if modified

This issue would begin life postponed on #2949209: Phase I: Allow drupalci.yml to influence the build, but we also want to be able to re-run tests against it to grab the generated build YML artifact.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#20 interdiff.txt618 bytesmile23
#20 2952984_20.patch973 bytesmile23
#19 2952984_19.patch528 bytesmile23
#17 2952984_17.patch1.46 KBmile23
#16 2952984_16.patch1.24 KBmile23
#15 2952984_15.patch1.23 KBmile23
#14 2952984_14.patch842 bytesmile23
#13 2952984_coverage-report-13.patch842 bytesMixologic
#10 2952984_coverage-report-10.patch546 bytesMixologic
#8 2952984_coverage-report.patch532 bytesmile23
#7 2952984_update_phpunit.patch405 bytesmile23
#2 2952984_2_add_drupalciyml.patch197 bytesmile23

Comments

Mile23 created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 2952984_2_add_drupalciyml.patch197 bytes

Adding a drupalci.yml file.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Postponed

Postponed on #2949209: Phase I: Allow drupalci.yml to influence the build

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

This claims it is still running, because pift is assuming that simpletest is going to run always. we'll need to address that assumption somehow.

Magically manipulated the database to shout.

jonathan1055’s picture

jonathan1055 commented
Title: Add drupalci.yml file » Add example drupalci.yml file

Altering title so that text of links to this issue are clearer, and more distinct from the core issue #2951843: Use drupalci.yml to fail test runs early

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

Pift is no longer reliant on there being some sort of testresults, so this works now.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Postponed » Needs review
StatusFileSize
new 2952984_update_phpunit.patch405 bytes

Let's try this out.

The patched build file tries to update phpunit/phpunit using composer, and then only runs unit tests.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_coverage-report.patch532 bytes

Maybe try a coverage report.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

container_command is a buildstep, not a buildphase, so its not finding the coverage at that indent level.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
StatusFileSize
new 2952984_coverage-report-10.patch546 bytes

Also bash variables need $'s, and also good to surround them with {}

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

I guess we need to change the documentation then. :-)

Also: no xdebug, so we don't get coverage: 

16:15:28 Warning:	The Xdebug extension is not loaded
16:15:28 		No code coverage will be generated.
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented

Right. and Im adding phpdbg over here: https://www.drupal.org/project/drupalci_environments/issues/2959249, because thats what I did on my plane ride out to here, was read all about code coverage.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
StatusFileSize
new 2952984_coverage-report-13.patch842 bytes

Here's a patch that demonstrates enabling xdebug so that you can run code coverage.

Also it demonstrates gathering code coverage with the phpdbg method as well.

I attempted to run it like this: php -dzend_extension=/usr/local/lib/php/extensions/no-debug-non-zts-20170718/xdebug.so ./vendor/bin/phpunit -c core/ --testsuite unit --group simpletest --coverage-text , but that failed on detecting that the extension was loaded in subordinate processes. Not sure if theres a way to cause php to inherit command line extension settings...

1) Drupal\Tests\simpletest\Unit\SimpletestPhpunitRunCommandTest::testSimpletestPhpUnitRunCommand with data set #0 (0, 'pass')
PHPUnit\Framework\Exception: PHP Fatal error:  Uncaught SebastianBergmann\CodeCoverage\RuntimeException: No code coverage driver available in /var/www/html/vendor/phpunit/php-code-coverage/src/CodeCoverage.php:1058
Stack trace:
#0 /var/www/html/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(148): SebastianBergmann\CodeCoverage\CodeCoverage->selectDriver()
#1 Standard input code(40): SebastianBergmann\CodeCoverage\CodeCoverage->__construct(NULL, Object(SebastianBergmann\CodeCoverage\Filter))
#2 Standard input code(110): __phpunit_run_isolated_test()
#3 {main}
  thrown in /var/www/html/vendor/phpunit/php-code-coverage/src/CodeCoverage.php on line 1058
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_14.patch842 bytes

Moved the coverage command to after run_tests, so we benefit from run_tests.sh installing newer PHPUnit version.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_15.patch1.23 KB

So yah, phpdbg isn't available on PHP 5.5 which is why it doesn't work on that build. Also, we can't combine --testsuite in PHPUnit < 6, which is a problem because we'd want to run both unit and kernel tests for coverage. (Functional tests don't actually cover code, they cover behavior.)

So basically if you use PHP 7 you can get a decent coverage report by re-running a bunch of tests. In an ideal world, we'd allow a non-zero exit code from container_command to represent a failed testbot build, and I think we're doing that in #2960534: Allow tasks to override build status without halting the build. If we can do that, then a container_command task could run all the coverage-oriented tasks, and run_tests could run functional and simpletest, to benefit from concurrency.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_16.patch1.24 KB

Oops... Missed adding kernel testsuite.

This build will break on PHP 5.5 and 5.6, but not PHP 7+, because only PHPUnit 6+ can take multiple testsuites as an argument.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_17.patch1.46 KB

Let's do the phpunit upgrade first.

Status: Needs review » Needs work

The last submitted patch, 17: 2952984_17.patch, failed testing. View results

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review
StatusFileSize
new 2952984_19.patch528 bytes

Over in #2897889: method/Class QueryFactory is deprecated we see that we're using a deprecated class, QueryFactory.

QueryFactory does @trigger_error(), so we should see fails on Examples HEAD test results. But we don't.

So here's a drupalci.yml file which does not suppress deprecation errors. Let's see what technical debt exists in Examples.

Edit... Er, make that @trigger_error() in core 8.6.x. Also a lot of other things seem to do that as well under 8.6.x :-) Followups to come.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new 2952984_20.patch973 bytes
new interdiff.txt618 bytes

Added some descriptive text.

Our follow-ups related to deprecations shown here: #2975898: Remove usages of @deprecated SafeMarkup methods (8.6.0) #2975897: Remove usages of @deprecated drupal_set_message() #2897889: method/Class QueryFactory is deprecated

Here's an Examples HEAD test run against 8.6.x: https://www.drupal.org/pift-ci-job/976164

It shows us that, by default, the testbot suppresses deprecations for contrib. This is by design: #2607260-80: [meta] Core deprecation message introduction, contrib testing etc.

So the failing 8.6.x tests here and that HEAD test against 8.6.x show us that if we want to not suppress deprecations, we need to do it in our drupalci.yml file.

And we do, so we do. :-)

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Title: Add example drupalci.yml file » Add example drupalci.yml file to un-suppress deprecations

  • Mile23 committed fd9fb85 on 8.x-1.x 
    Issue #2952984 by Mile23, Mixologic, jonathan1055: Add example drupalci....
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Fixed

Since we want to see all those deprecations and be exemplary, we'll add this drupalci.yml file.

Any other ideas about what to demonstrate in a drupalci.yml file? File a follow-up please!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.