dev/core#938 - remove duplicate lines #15216
Conversation
|
(Standard links)
|
|
yep makes sense probably a copy and paste issue |
| @@ -349,9 +349,6 @@ public function testCreate($testEntityClass, $createParams, $expectedContent) { | |||
| 'return' => ['content'], | |||
| ]); | |||
| $this->assertEquals($expectedContent, $getResult2['values'][$fileId]['content']); | |||
| foreach (['id', 'entity_table', 'entity_id', 'url'] as $field) { | |||
| $this->assertEquals($createResult['values'][$fileId][$field], $getResult['values'][$fileId][$field], "Expect field $field to match"); | |||
| } | |||
There was a problem hiding this comment.
Hmm, yeah, I think the intent was to ensure that the get always returned essential fields (id, entity_table, entity_id) even if the return list didn't specify them, and the old snippet looks like a copy-paste error. (s/$getResult/$getResult2/)
Good point, though, that checking $getResult is effectively superfluous here.
There was a problem hiding this comment.
Ah yes I see it does return those even if you only ask for content. So I can put it back replacing with the getResult2 and removing url, and then adding a comment that that's why it's there.
There was a problem hiding this comment.
@demeritcowboy maybe replicate it what we did with the url above as well
b6890e0 to
5f3fd5f
Compare
Overview
See https://lab.civicrm.org/dev/core/issues/938#note_22580. There's some duplicate lines that were never noticed before because the only time they would fail was when the lines above it would fail, but that meant the test would end before hitting them. Now that the lines above it are worked-around this comes up.
Before
Test can still fail intermittently because of duplicated lines.
After
Lines removed.
Technical Details
These duplicate lines are comparing the results of the previous
getcall a second time. My guess is they were copy and paste.Comments