When #243532: Catch notices, warnings, errors and fatal errors from the tested side is applied, the ImportOPMLTestCase generates the following notices when posting to content/aggregator/add/opml:

Exception Notice aggregator.admin.inc 364
Undefined index: TEXT

CommentFileSizeAuthor
#6 56_passes_0_fails_and_0_exceptions.patch614 byteschx
#3 another_notice_down.patch1.78 KBboombatower
#1 another_notice_down.patch671 byteschx

Comments

chx’s picture

chx commented
Status: Active » Needs review
StatusFileSize
new another_notice_down.patch671 bytes
boombatower’s picture

boombatower commented
Status: Needs review » Needs work

This patch creates:

Verifying that two distinct feeds were added.	Other	aggregator.test	588	ImportOPMLTestCase->submitImportForm()
boombatower’s picture

boombatower commented
Status: Needs work » Needs review
StatusFileSize
new another_notice_down.patch1.78 KB

It seems that 3 feeds are actually inserted, but that 2 used to have a blank title so 1 was thrown out as Duplicate title. The following OPML is used in upload

<opml version="1.0">
  <head></head>
  <body>
    <!-- First feed to be imported. -->
    <outline text="{$feeds[0]['title']}" xmlurl="{$feeds[0]['url']}" />

    <!-- Second feed. Test string delimitation and attribute order. -->
    <outline xmlurl='{$feeds[1]['url']}' text='{$feeds[1]['title']}'/>

    <!-- Test for duplicate URL and title. -->
    <outline xmlurl="{$feeds[0]['url']}" text="Duplicate URL"/>
    <outline xmlurl="http://duplicate.title" text="{$feeds[1]['title']}"/>

    <!-- Test that feeds are only added with required attributes. -->
    <outline text="{$feeds[2]['title']}" />
    <outline xmlurl="{$feeds[2]['url']}" />
  </body>
</opml>

Attached patch changes it to expect 3. Please confirm that I'm making sense.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

Hm. Don't think so. From how I read that OPML file, the only two rows which ought to be added are the ones labeled first feed and second feed. If 3 are getting through, one of the assertions before that (or the code it's testing) is buggy, since they're all checking specifically for that.

What rows end up in the database?

catch’s picture

catch
he/him
Primary language English
commented
Priority: Normal » Critical

subscribe/critical etc.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 56_passes_0_fails_and_0_exceptions.patch614 bytes

Hummm, humm.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work

http://webchick.net/patch-reviewers-are-not-clairvoyant :P

chx’s picture

chx commented
Status: Needs work » Needs review

I have nothing to say. If you want to use two array elements, check for the existence of both. That's exactly what the code does. I thought about 20 chars of new code did not require a description of length of War and Peace.

chx’s picture

chx commented 
 <outline xmlurl="{$feeds[2]['url']}" />

no TEXT but passed the condition and throwed a notice.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Fixed

Thank you! :)

Committed to HEAD. :D

Status: Fixed » Closed (fixed)

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