Problem/Motivation

(Rewritten after request on comment #10.)

We need to test top-level tab creation separately from tray creation, when using hook_toolbar() to do so.

The current tests use toolbar_test.module to only test returning both a top-level tab and a tray together, so not covering the full range of behaviours.

Proposed resolution

Because any given module's hook_toolbar() always returns the same array of data, we need multiple implementations to test multiple scenarios.

  1. Rename module toolbar_test to toolbar_test_tab_and_tray.
  2. (Maybe run this through coder-review when doing so, as e.g. indentation seems a little off?)
  3. Clone it to create two new modules toolbar_test_tab and toolbar_test_tray, and edit down their hook_toolbar() implementations.
  4. Similarly with ToolbarHookToolbarTest, rename and clone, to test now three different toolbar_test_* modules.

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Update the issue summary Instructions Yes
Create a patch Instructions
Add automated tests Instructions

User interface changes

None: tabs and trays are only added during running of tests.

API changes

None: new tests are only autodiscovered during running of tests.

Comments

benjifisher’s picture

@jessebeach:

In #1137920: Fix toolbar on small screen sizes and redesign toolbar for desktop you have been saying that "benjifisher is working on" this issue. Not yet. I will get to it soon. (Too many comments and too many followers on that issue, and I do not want to add to the overload.)

If anyone else wants to work on it, I promise to post a comment when I start. I hate duplicated effort.

I can exercise hook_toolbar() and hook_toolbar_alter(). I would appreciate help, advice, or pointers on how to test the javascript.

jessebeach’s picture

No need to address JavaScript testing, just simpletests of PHP. We'll be adding JS tests after code freeze.

Shyamala’s picture

Issue tags:+toolbar-followup

editing tags

YesCT’s picture

I was looking for a simpletest from when the toolbar went in, that tested for icons, so I could copy it for this language issue:
#1848552: Toolbar icons disappear with translated menu

It sounds like there are not tests yet?

benjifisher’s picture

I am working on rewriting hook_toolbar() in #1847198: Update the structure returned by hook_toolbar(). I will submit patches here when that work is done. For now, I have set up a sandbox project for a testing/example module for the toolbar hooks: http://drupal.org/sandbox/benji/1862108.

I would like to have help on the sandbox project from someone who knows CSS, icons, and javascript better than I do.

benjifisher’s picture

Status:Active» Postponed

The existing tests are being updated as part of #1847198: Update the structure returned by hook_toolbar(). I think it makes sense to wait until that issue is resolved before adding any more, so I am marking this issue Postponed.

According to the discussion in #1532612: Move Examples Project into Drupal core, we should do much more than the tests suggested in the issue summary. We should test every edge case we can imagine and do our best to break the API.

benjifisher’s picture

Issue summary:View changes

Updated issue summary.

mgifford’s picture

Issue summary:View changes
Status:Postponed» Active
Related issues:+#1847198: Update the structure returned by hook_toolbar()
Wim Leers’s picture

Title:Establish test coverage for toolbar module.» Expand test coverage for Toolbar module: test a top-level tab without a tray
Issue tags:+Novice, +php-novice

Add a top-level tab with a tray

We have this now, was added in #1847198: Update the structure returned by hook_toolbar().

We don't have

Add a top-level tab

yet.

Updating accordingly.

Wim Leers’s picture

Issue summary:View changes
YesCT’s picture

Issue summary:View changes
Issue tags:+Needs issue summary update

I think this needs an issue summary update in order for a new contributor to understand what needs to be done.

https://drupal.org/core-mentoring/novice-tasks helps people decide what is a novice task and what is not

jp.stacey’s picture

Issue tags:+SprintWeekend2016

Looking at this as part of Sprint Weekend 2016.

jp.stacey’s picture

Issue summary:View changes
Status:Active» Needs work
Issue tags:-Needs issue summary update+Needs patch

Updated issue summary. I'm happy to work on this if the proposed solution looks right, but given I've expanded a lot on the original description it would be good to get some other eyes on it before I go off on a tangent!