Can we please allow themes to participate in testing? There's no "Automated testing" tab on theme projects like there is on modules. I get that we don't have the infra for things like JS or Behat/Mink testing, but that doesn't mean they still can't run php based unit and web based tests.

This would be extremely useful for complex base themes like https://www.drupal.org/project/bootstrap, especially now that in 8.x it utilizes extensive OO code.

Comments

markcarver created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented

Are you sure it is not possible to add tests to themes? Just from reading \Drupal\simpletest\TestDiscovery::getExtensions I would assume it does

Mixologic’s picture

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

I think what he means is that there is no 'automated testing' tab on project_theme types on drupal.org, and he'd like there to be one. Can you test themes with run-tests.sh? because, right now, that is the only currently supported mechanism. We'll be adding new test types soon, but we're not running phpunit (unless via run-tests.sh).

I.e. does bootstrap have any tests we can check this with?

markhalliwell’s picture

markhalliwell
Primary language English
commented
Related issues: +#2138693: Provide UnitTests for theme, +#2138733: run-test.sh needs full path for --file

\Drupal\simpletest\TestDiscovery::getExtensions

That's promising.

I think what he means is that there is no 'automated testing' tab on project_theme types on drupal.org, and he'd like there to be one.

Correct.

Can you test themes with run-tests.sh?

To be quite honest, tests aren't my thing, but I know they're important. I really haven't put a whole lot of effort into something that isn't actively supported yet. I know last time I went down this route, there was an issue where the full path wasn't used in --file properly (see related issue).

I.e. does bootstrap have any tests we can check this with?

Not currently, no. I haven't even begun to put effort into creating tests because there hasn't been any support of it. That being said, there is a mountain of things that could be tested now that the entire codebase is OO.

The closest I have come to this issue are the related issues, which was before or rather superseded by DrupalCI.

markhalliwell’s picture

markhalliwell
Primary language English
commented

I was able to successfully run the test using the patch at #2138693-7: Provide UnitTests for theme and the following CLI:

sudo -u _www php ./core/scripts/run-tests.sh --sqlite /tmp/d8.dev-sqlite --directory themes/bootstrap --verbose

---

That being said, I'm not entirely sure if using --directory qualifies. I tried using the --class, but it couldn't find it.

markhalliwell’s picture

markhalliwell
Primary language English
commented

Ok, so I think it wasn't finding the class because the @group annotation was missing. Fixed that in #2138693-8: Provide UnitTests for theme.

The following seems to be working now:
sudo -u _www php ./core/scripts/run-tests.sh --class "Drupal\bootstrap\Tests\BootstrapUnitTest"

technikh’s picture

technikh commented

Also allow distributions to be testable.
This Drupal commons distribution patch on https://www.drupal.org/node/2315729 doesn't show test results

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Project: DrupalCI: Drupal.org Testing Infrastructure » Project issue file test
Version: » 7.x-3.x-dev
Component: Miscellaneous » Testing Triggers

Distributions are another thing entirely. I've opened a new issue for that: #2779227: [Policy] Distribution testing

markhalliwell’s picture

markhalliwell
Primary language English
commented

Bump... should this be moved back to Drupal CI?

drumm’s picture

drumm
he/him
Location NY, US
commented

Dependencies of themes are currently not parsed by project_dependency. I’m not sure there’s much of a reason not to do that, other than potential for adding noise - themes or subthemes with colliding names being chosen for non-namespaced dependencies. However, that’s planned to be replaced with Composer for DrupalCI’s dependency resolution. project_dependency will still be used for Drupal.org’s Composer shim and for showing dependencies on Drupal.org.

I think the next best steps are

  • Check if installing themes with Composer works well.
  • Try it out with a local instance of DrupalCI
markhalliwell’s picture

markhalliwell
Primary language English
commented

I'm really not sure what composer has to do with this...

This is, primarily, about adding (re-enabling, it was removed a while back) the "Automated testing" tab on theme project pages.

I honestly could care less about "dependency" issues at this moment. That can be handled in future issues.

Getting a project (base theme), that has no dependencies, to be "testable" is a massive win IMO... especially for 8.x where there is autoloading/discoverability of test classes.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Project: Project issue file test » DrupalCI: Drupal.org Testing Infrastructure
Version: 7.x-3.x-dev »
Component: Testing Triggers » Policy
Category: Feature request » Plan

It's more complicated than that. Adding the option back to the automated testing tab isn't going to make your theme testable.

In order to test anything we have to construct a codebase to test. For modules this means drupal core, and any other module dependencies that are required. We're currently using project_dependency to make that happen, but we're working to get away from that entirely and use composer to build out the codebase, hence why its important that we can install your theme using composer.

Additionally, for themes, we'll need to know what additional build process we might need to support. Do we need to run bower? npm? Do we need to download additional assets or libraries? What do we have to do to get it into a *testable* state. How many themes on d.o. even *need* additional build process support? Obviously bootstrap doesn't need much of anything besides a copy of core lying around.

For that matter what other *types* of tests should we support for themes, is there demand for javascript tests?

So yeah, this is probably a drupalci issue right now, and its really a meta of "what do we need to do to support theme testing, and whats the roadmap to get us there?"

markhalliwell’s picture

markhalliwell
Primary language English
commented
Related issues: +#2547797: Add composer.json to Bootstrap

Ok. I added the necessary composer.json file to the base theme. It needed to get done anyway, was just confused as to why it was needed here, thanks for clearing that up.

Additionally, for themes, we'll need to know what additional build process we might need to support. Do we need to run bower? npm?

Ideally, yes, it would be fantastic if Drupal CI could execute npm install and npm test. TBH, however, that is an entirely separate issue/feature.

As it stands now, there are simply a multitude of normal OO PHP code that could currently be unit tested in the base theme: http://drupal-bootstrap.org/api/bootstrap/classes. That is primarily what I'm discussing here.

For that matter what other *types* of tests should we support for themes, is there demand for javascript tests?

Similar to above. Yes, it'd be nice to have other JS based testing suites available. That's one of the huge advantages of hosting code on GH. I think implementing a file like .travis.yml that specifies the environment requirements would be preferable. Whether or not that is open ended to allow before/after scripts (to install other tools), I'm not sure about.

For now, I think simply opening up the ability to unit test PHP code and [potentially] nodejs/npm testing would be fine.

markhalliwell’s picture

markhalliwell
Primary language English
commented

Bump... stable releases have composer.json now. What's next?

markhalliwell’s picture

markhalliwell
Primary language English
commented

Bump... again...

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Active » Postponed

We can do this, but we need PIFT to talk to us better first.

Postponing on https://www.drupal.org/project/project_issue_file_test/issues/2853889

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Related issues: +#2853889: Send improved project metadata to testbots
markhalliwell’s picture

markhalliwell
Primary language English
commented
Parent issue: » #2949209: Phase I: Allow drupalci.yml to influence the build
Related issues: -#2853889: Send improved project metadata to testbots +#2909467: Start sending build.yml
markhalliwell’s picture

markhalliwell
Primary language English
commented
Title: Allow themes to be testable » [PP-1] Allow themes to be testable
Component: Policy » User interface
Category: Plan » Task
Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Postponed » Needs review

https://www.drupal.org/node/259843/qa can now be configured.

We havent really ran this through a full test because we dont have something exactly to test with, but its probably better to hand it to you to see if it works.

markhalliwell’s picture

markhalliwell
Primary language English
commented

w00t! Testing now (well, queued hehe)!

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs review » Needs work

I've created the following branch tests:
https://www.drupal.org/node/259843/qa

How does one remove a test? I accidentally created one for 8.x-4.x.

#2138693: Provide UnitTests for theme cannot seem to get patch validation:
https://dispatcher.drupalci.org/job/drupal8_contrib_patches/24672/console

7.x-3.x says it doesn't have a composer.json file, even though it does:
http://cgit.drupalcode.org/bootstrap/tree/composer.json?h=7.x-3.x

Seems to be using https://packages.drupal.org/8... even though the test is configured for 7.x. Not sure if that has anything to do with it.

markhalliwell’s picture

markhalliwell
Primary language English
commented

How does one remove a test? I accidentally created one for 8.x-4.x.

Turns out you have to wait for the test to complete and then an “edit” link appears which lets you then delete from there. Not very intuitive, just FYI.

Mixologic’s picture

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

Yeah, that whole UI has kinda been a lot of little incremental things, and could probably, someday, be more intuitive.

The 7.x test was breaking because I was setting some wrong keys, and have since redeployed to take that into account. The branch tests were also running *all of core* which they werent supposed to.

So, sorry to announce this to you before I'd finished ironing out the new deployment kinks. I've requeued some branch tests to see where we're at now.

markhalliwell’s picture

markhalliwell
Primary language English
commented

No need to apologize :D I was just a bit confused as I don't typically go this area of a project that much.

I'm just very happy that this is finally starting to happen! This is like an early Christmas present lol

It seems the branches are now "passing" https://www.drupal.org/node/259843/qa

They seem to correctly indicate now that the current branch code doesn't have any tests to run.

Tried to retest #2138693: Provide UnitTests for theme, but it does seem there needs a little bit more work there.

Mixologic’s picture

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

Okay, I've found a few spots in both pift and drupalci that are expecting 'module' vs a more generic extension. but now we've got a nice build.yml file we can test with that should help out.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
commented
Status: Needs work » Fixed

Fixed all those spots, themes seem to be testable again.

markhalliwell’s picture

markhalliwell
Primary language English
commented

Awesome!!! Thank you again @Mixologic for putting so much love into this! This is absolutely fantastic work!

Status: Fixed » Closed (fixed)

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