Undefined index warning in "Available Updates List" with hidden=TRUE base themes

Hmmm, looks like it could be a bug in Drupal - I tracked it down to hidden = TRUE, which I'm using in adaptivetheme.info to hide the base theme from end users (such as on the Appearance page). We're meant to be able to use this in Themes in Drupal 7, see http://drupal.org/node/542202 which states this can be used for themes (maybe not base themes?).

I'm moving this to the update module queue to get some feedback.

I have removed the hidden = TRUE flag from the info file and has been committed to CVS, I'll be releasing an updated version of the theme very soon, just trying to sort out the skinr module stuff which is holding up a release.

Got this message for Zen to.

drbeaker:

I think you'll need to manually check for updates once for this to take effect -- (I think it's relying on insufficient cached data). Go to Reports >> Available Updates >> Updates (it's the tab in the upper-right) >> Check manually.

Please report the results.

Sorry, meant to also say that patch seems to work cleanly. No longer getting the notice error.

drbeaker: This should fix your issue in #7.

Attached patch addresses the note in #11. The main change is moving a !$file->status if statement lower down to allow building a list of enabled subthemes in more cases (i.e., always).

This might still need some more work, as the list of includes / depends / required by might be impacted by this.

Reuploading to preserve the random testbot fail in #12.

Hi All,

I got a similar error message running the update script on a Drupal 7.0 instance.

Ran updates:
http://(my site).com/#overlay=admin/reports/updates

Got error message:
Notice: Undefined index: boron in theme_update_report() (line 201 of [path from root]/modules/update/update.report.inc).

Took a look at the 'update.report.inc' but can't see what might be triggering the error. Not on line 201 or anywhere else in the file.

I'm stumped.

- dave myers

#14: 986888-14-fix_undefined_index_with_hidden_base_themes.patch queued for re-testing.

Subscribing here too. This is showing for all Fusion core and sub themes.

And now it needs a reroll again. :)

Tagging as novice for the task of rerolling the Drupal 8.x patch on account of #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

The last submitted patch, 986888-now-with-core.patch, failed testing.

#25: 986888-now-with-core.patch queued for re-testing.

#1346772: StatisticsLoggingTestCase->testLogging() fails with clean URLs in some environments again... someone please RTBC that patch before I hurt testbot. :)

The patch looks good to me. We should be able to add tests for this.

I am getting this on Fusion_Core:
"Notice: Undefined index: fusion_core in theme_update_report() (line 200 of /var/www/patchxxxxxxx/modules/update/update.report.inc)."

It's been a long time since this was an active thread - did I miss something ?
I am current on all code & modules as of this date...

#25: 986888-now-with-core.patch queued for re-testing.

Looking into writing an automated test.

I think we are fixing this first in 8.x and then in 7.x.
@carsonblack, it's been awhile. If you want to tackle this again, just post a note. :) Thanks for trying. Tests can be tricky. Otherwise it's available.

I rerolled the patch from post 25, "986888-now-with-core.patch" to apply cleanly to commit 77f0384.

confirming that patch in #45 removes the warning for base themes with the hidden = TRUE.

status update for automated tests, which are still needed.

Thanks @bdone!

One thing we want to do in general is assert the correct, expected behavior rather than merely asserting that there is no error message. Otherwise, it could be broken in a different way but still pass the test.

Another thing to keep in mind is that the error reporting settings can vary from site to site, so if we are using assertText() to check whether an error was reported, we need to make sure the error would be reported. See ErrorHandlerTest for an illustration of how the error handling settings can affect what messages are displayed. In general, I recommend against asserting the absence of an error message, but let's start by looking at the test-only patch (like we discussed in IRC) to make sure it provides coverage.

Another thing we could try would be to test the function directly--call update_process_info_list() within the test method, instead of visiting a page where it is called with the browser. That way, the error should hopefully be raised in the test environment rather than the child environment, which would cause the test to fail.

1. +++ b/core/modules/update/lib/Drupal/update/Tests/UpdateContribTest.phpundefined
@@ -262,6 +262,37 @@ function testUpdateShowDisabledThemes() {
+    /**
+   * Tests that hidden base themes do not report a specified undefined
+   * index error in the available updates report.
+   */
+  function testUpdateHiddenBaseThemeError() {

   The summary should be one line of 80 characters or fewer. I'd suggest broadening the method's purpose a little to test hidden base themes in general, like:
   

   /**
* Tests updates with a hidden base theme.
*/
function testUpdateHiddenBaseTheme() {
2. +++ b/core/modules/update/lib/Drupal/update/Tests/UpdateContribTest.phpundefined
@@ -262,6 +262,37 @@ function testUpdateShowDisabledThemes() {
+    $this->assertNoText(t($message), t('The hidden base theme, update_test_basetheme, did not produce error message: !message', array('!message' => $message)));

   In general, assertion message texts should not use t(). See: http://drupal.org/simpletest-tutorial-drupal7#t

   Also, for assertText() and assertNoText(), the default message is usually sufficiently clear, so we can actually leave off the $message parameter here.

@bdone interdiff please (unless it's a reroll that cannot make one because of merges or conflicts)

okay, got a bit confused about which interdiff to provide here. attaching one between 47 and 49 as suggested by @Cottser in #drupal. thanks for helping clear it up!

hopefully this incorporates feedback from @YesCT correctly...

• 986888-54-tests.patch: a duplicate of tests patch in #51.
• 986888-54-complete.patch: original patch from #44, including #54 tests patch.
• interdiff-54-51.txt: an interdiff from #54 to #51.

ok. we are getting completely confused with all these patches. @bdone dont worry much. we will sort out interdiffs on the next issue. :)

I went through this, and test and complete patch in 54 are ok.

@catch how about adding a @todo to convert to those once the are fixed? Or would that conflict with those issues and slow down them?

Those issues are not moving very quickly, and have their own dependencies.

Those issues or their dependencies don't look back portable to d7, would we still use an approach like this one in d7?

I suppose waiting a week or two to see how feature completion shakes down feb 18 in those issues is not too long to wait.

Updating tags (this has tests, and to revisit)

Committed/pushed to 8.x, thanks!

The last submitted patch, 986888-61-available-updates-list.patch, failed testing.