Problem/Motivation

#2298667: Code review for report generation Identified that Migrate module's NodeRevisionTest and NodeRevisionByNodeTypeTest are broken and don't test anything. they have been marked as incompletely and we should fix them and provide proper coverage.

Proposed resolution

Probably the proper fix FakeSelect to support the query needed by the tests.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
phpunit-strict-followup.patch12.06 KBneclimdul
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Status: Active » Needs work

better reflect status.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Title: Fix migrate NodeRevisionTest » Fix Various Unit Tests In the Migrate Module
Component: phpunit » migration system
Issue summary: View changes
Issue tags: +PHPUnit
Related issues: +#2298667: Code review for report generation
neclimdul’s picture

neclimdul
He/Him
CreditAttribution: neclimdul commented
Title: Fix Various Unit Tests In the Migrate Module » Fix migrate NodeRevisionTest
Issue summary: View changes

Only NodeRevisionTest is actually broken/incomplete currently.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Title: Fix migrate NodeRevisionTest » Fix migrate NodeRevisionTest & NodeRevisionByNodeTypeTest
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

There is an issue for those two incomplete tests: #2299795: Fix migrate NodeRevisionTest. Wouldn't that actually be critical?

I don't see why it should be critical ... its valid to write incomplete tests, its just a detail that we don't use that in other areas.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Fixed

The patch here is substantially the same as the existing test, it looks like it simply has fields on the nodes. Probably can be closed.

Yes, I checked with neclimdul on IRC, who also agrees with closing this issues.

Status: Fixed » Closed (fixed)

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