Add test coverage for Olivero. We can use Drupal\FunctionalTests\Theme\BartikTest as an example for this.

CommentFileSizeAuthor
#16 interdiff-3135511-15-16.txt468 bytesbrianperry
#16 3135511-16.patch4.31 KBbrianperry
#15 interdiff-3135511-5-11.txt2 KBbrianperry
#15 3135511-15.patch4.29 KBbrianperry
#11 3135511-11.patch4.56 KBbrianperry
#11 interdiff-3135511-5-11.txt2.93 KBbrianperry
#5 interdiff-3135511-2-5.txt370 bytesmsuthars
#5 3135511-5.patch2.82 KBmsuthars
#2 3135511-2.patch2.82 KBJayKandari
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JayKandari created an issue. See original summary.

JayKandari’s picture

JayKandari
Primary language Hindi
CreditAttribution: JayKandari as a volunteer and at QED42 for Drupal India Association commented
Assigned: JayKandari » Unassigned
Status: Active » Needs review
FileSize
3135511-2.patch2.82 KB

* Created a basic functional test which checks if Olivero theme is installed correctly & checks if base.css is loaded or not.
* This also required updating the schema.yml file.
* Test required enabling of additional modules.
* Olivero module is installed in setup phase.

A sample local run in core 8.9.x : 

❯ vendor/bin/phpunit -c core --group olivero -v
PHPUnit 6.5.14 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.5
Configuration: /Users/jaykandari/work/drupal8/core/phpunit.xml

Testing 
.                                                                   1 / 1 (100%)

Time: 24.61 seconds, Memory: 981.00MB

OK (1 test, 4 assertions)

HTML output was generated
http://localhost:8000/sites/simpletest/browser_output/Drupal_Tests_olivero_Functional_OliveroTest-4-32727592.html

Attached the patch. Kindly Review.

proeung’s picture

proeung
she/her
CreditAttribution: proeung as a volunteer commented

@JayKandari Thanks for creating this issue and submitting a patch. I tested this patch on core 8.9.x and it works. Now we just need to test this against Core 9.0.0 to make sure the test runs correctly.

msuthars’s picture

msuthars
Location Pune
CreditAttribution: msuthars as a volunteer and at QED42 for Drupal India Association commented
Version: » 8.x-1.0-alpha1
Assigned: Unassigned » msuthars
Status: Needs review » Needs work

@JayKandari Thanks for the patch. Tested with Drupal Core 9.0.0 and found a depreciation issue.

1x: Declaring ::setUp without a void return typehint in Drupal\Tests\olivero\Functional\OliveroTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

Working on this depreciation issue.

msuthars’s picture

msuthars
Location Pune
CreditAttribution: msuthars as a volunteer and at QED42 for Drupal India Association commented
Assigned: msuthars » Unassigned
Status: Needs work » Needs review
FileSize
3135511-5.patch2.82 KB
interdiff-3135511-2-5.txt370 bytes

Updated the patch. The depreciation issue is fixed.

../vendor/bin/phpunit ../themes/contrib/olivero/tests/src/Functional/OliveroTest.php
PHPUnit 8.5.5 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\olivero\Functional\OliveroTest
.                                                                   1 / 1 (100%)

Time: 49.72 seconds, Memory: 4.00 MB

OK (1 test, 4 assertions)
brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented

Running this on 9.0 I get the following error:

vendor/bin/phpunit web/themes/contrib/olivero/tests/src/Functional/OliveroTest.php --verbose
PHPUnit 8.5.6 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.3.11
Configuration: /Users/brian/repos/olivero-sandbox-d9/phpunit.xml

Testing Drupal\Tests\olivero\Functional\OliveroTest
E                                                                   1 / 1 (100%)

Time: 1.29 seconds, Memory: 6.00 MB

There was 1 error:

1) Drupal\Tests\olivero\Functional\OliveroTest::testBaseCssFileAvailable
Undefined index: value

/Users/brian/repos/olivero-sandbox-d9/web/core/includes/install.core.inc:2294
/Users/brian/repos/olivero-sandbox-d9/web/core/includes/install.core.inc:1079
/Users/brian/repos/olivero-sandbox-d9/web/core/includes/install.core.inc:693
/Users/brian/repos/olivero-sandbox-d9/web/core/includes/install.core.inc:568
/Users/brian/repos/olivero-sandbox-d9/web/core/includes/install.core.inc:118
/Users/brian/repos/olivero-sandbox-d9/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:286
/Users/brian/repos/olivero-sandbox-d9/web/core/tests/Drupal/Tests/BrowserTestBase.php:576
/Users/brian/repos/olivero-sandbox-d9/web/core/tests/Drupal/Tests/BrowserTestBase.php:400
/Users/brian/repos/olivero-sandbox-d9/web/themes/contrib/olivero/tests/src/Functional/OliveroTest.php:28
/Users/brian/repos/olivero-sandbox-d9/vendor/phpunit/phpunit/src/Framework/TestResult.php:691

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

Tried a few things but can't seem to figure out the cause.

msuthars’s picture

msuthars
Location Pune
CreditAttribution: msuthars as a volunteer and at QED42 for Drupal India Association commented

@brianperry I run the test case with Drupal 9 and it works as expected.

/var/www/html/core$ ../vendor/bin/phpunit ../themes/contrib/olivero/tests/src/Functional/OliveroTest.php --verbose
PHPUnit 8.5.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.5
Configuration: /var/www/html/core/phpunit.xml

Testing Drupal\Tests\olivero\Functional\OliveroTest
.                                                                   1 / 1 (100%)

Time: 2.6 minutes, Memory: 4.00 MB

OK (1 test, 4 assertions)

HTML output was generated
https://drupal9.ddev.site/sites/simpletest/browser_output/Drupal_Tests_olivero_Functional_OliveroTest-3-44451399.html

I'm using ddev and sometimes I get that test case issue (#6) when my ddev stops. So I have to start it again and it works for me.

steinmb’s picture

steinmb CreditAttribution: steinmb as a volunteer commented
Version: 8.x-1.0-alpha1 » 8.x-1.0-alpha3

Tests are always good :) Have not run the test though we should make sure we are testing against latest version. Is there a reason there is no dev. release to lock issues to so we don't have to chase versions inside issues?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented

This is documented as a must have in #3111409: Add new Olivero frontend theme to Drupal 9.1 core as beta

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented
Assigned: Unassigned » brianperry

My previous issues running this test were related to my VM configuration. All good now.

Grabbing this one and am working on adding some additional test coverage. First off I'm borrowing select tests from or inspired by the tests for Bartik (which the original patch covers) and Claro.

Beyond that, is there anything Olivero specific that we want to make sure we're testing? I'm sure there are a number of things we could add here, but we'll also need to be mindful of making sure the tests don't take too long to run.

Expect to have a patch for review in advance of the Monday meeting.

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented
Assigned: brianperry » Unassigned
FileSize
interdiff-3135511-5-11.txt2.93 KB
3135511-11.patch4.56 KB

Attached a patch that adds some more test coverage. Still a little new to PHPUnit browser tests, so open to any feedback.

> phpunit web/themes/contrib/olivero/tests/src/Functional/OliveroTest.php
PHPUnit 8.5.8 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\olivero\Functional\OliveroTest
...                                                                 3 / 3 (100%)

Time: 2.12 minutes, Memory: 6.00 MB

OK (3 tests, 26 assertions)

Based on the tests that these were modeled after, this file probably has to move to core/tests/Drupal/FunctionalTests/Theme when Olivero makes it into core.

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented

I'm also working on some Nightwatch tests as well. Don't quite have a patch ready to share, but thinking it might make sense for it to be a separate issue anyway so we can separate our testing tech stack concerns.

Example of what I think would be useful as a Nightwatch test: testing mobile menu visibility.

Status: Needs review » Needs work

The last submitted patch, 11: 3135511-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

steinmb’s picture

steinmb CreditAttribution: steinmb as a volunteer commented
Version: 8.x-1.0-alpha3 » 8.x-1.x-dev
brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented
FileSize
3135511-15.patch4.29 KB
interdiff-3135511-5-11.txt2 KB

Updated tests to reference new css location and remove image style test. Also addressed the formatting things codesniffer was complaining about.

brianperry’s picture

brianperry
he/him
CreditAttribution: brianperry at Bounteous commented
Status: Needs work » Needs review
FileSize
3135511-16.patch4.31 KB
interdiff-3135511-15-16.txt468 bytes

Hey look - the tests passed :)

Attaching one more patch to address the coding standards issues mentioned in the CI results.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Needs review » Reviewed & tested by the community

This looks great! Thanks for all of the hard work on this!

  • mherchel committed be7caa8 on 8.x-1.x authored by brianperry 
    Issue #3135511 by brianperry, msuthars, JayKandari: Create basic test...
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
CreditAttribution: mherchel at Lullabot commented
Status: Reviewed & tested by the community » Fixed

Committed. Thanks!

Status: Fixed » Closed (fixed)

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