Port basic test infrastructure for mapper [#623452]

Port basic testing infrastructure from Feed Element Mapper http://drupal.org/project/feedapi_mapper to Feeds. The two versions 1.x and 2.x should be pretty much the same. Let's use 2.x for consistency.

Comment	File	Size	Author
#5	623452-5_mapper_tests.patch	15.4 KB	alex_b
#5
#4	623452.patch	10.4 KB	pbuyle
#4
#3	623452.zip	3.98 KB	pbuyle

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Anonymous (not verified) CreditAttribution: Anonymous commented 6 November 2009 at 13:22

I had a look at this yesterday. But I don't know where to start.

I'm willing to do the grunt work and even some fussing around if someone can give me a hint of how I should start.

Is the goal to port FeedApiMapperBasicTestCase to Feeds so that existing feedapi_mapper tests can port easily?

Comment #2

alex_b CreditAttribution: alex_b commented 9 November 2009 at 18:43

@bangpound:

The goal is to create all shared functionality necessary to create mapper tests easily. Some of this functinality is already present in feeds/tests/feeds.test.inc. Some may still need to be ported from feedapi/tests while some existing functions in feeds/tests/feeds.test.inc may need some adjustments to make them better suitable for mapping tests. I have not looked deeper into the issue. I have not written the feedapi mapper tests and it may turn out that _all_ of the required 'shared functionality' may already be present in Feeds.

You may actually want to start by porting the mapper test for CCK integration (feedapi_mapper/tests/feedapi_mapper_content.test) and see whether there is functionality missing in the Feeds testing infrastructure that needs to be ported from FeedAPI Mapper.

Comment #3

pbuyle CreditAttribution: pbuyle commented 23 November 2009 at 11:25

Status:

Active

» Needs review

File	Size
623452.zip	3.98 KB

Here is my port of the CCK integration test.

Most of the functionalities were already in FeedsWebTestCase. The only required method that was missing is createContentType. It has been lightly refactored to ease the addition of types specific widget in sub-classes. I've also added an assertCCKFieldValue method to check the value of a CCK field from a node edit page. Checking field values from the edit page seems better then from node view pages (/node, /node/%nid, etc.). This method use two separates methods the get the form fields names and values for a given CCK field with support for complex ones (CCK fields with more than one form field).

Comment #4

pbuyle CreditAttribution: pbuyle commented 23 November 2009 at 16:35

File	Size
623452.patch	10.4 KB

Here is an updated version as a .patch file. It contains a fix for assertCCKFieldValue and allow to define fields settings in createContentType.

Comment #5

alex_b CreditAttribution: alex_b commented 24 November 2009 at 04:56

File	Size
623452-5_mapper_tests.patch	15.4 KB

Awesome. Nice work. Some adjustments:

- moved setSettings() up into FeedsWebTestCase, in a subsequent iteration we should make other tests make use of setSettings(), maybe we can find a nicer name ;-)
- killed some white space
- removed admin_user and required_modules from FeedsMapperTestCase - they seemed to be unused.
- removed content-taxonomy.csv - I take it that's to be added with taxonomy mapper.
- add a second row to content.csv and tests for it - you never know
- removed default mapping from FeedsNodeProcessor.inc - it corrupted addMapping(), further it broke with other parsers than syndication parsers.

All tests passing on my side.

mongolito404: could you take another good look at this patch, review my changes and then give me a thumbs up before I commit?

Thanks.

Comment #6

pbuyle CreditAttribution: pbuyle commented 24 November 2009 at 08:26

The last patch is good for me.

- I planned setSetting for FeedsWebTestCase but I did not want to go outside the scope of mapper testing with my patch
- required_modules is a avatar for an attempt to ensure feeds and content modules in setUp. It didn't work at first try, so I removed it. I may re-try later.
- content-taxonomy.csv is my test case for content taxonomy support (see #637334: Content Taxonomy Mapper)