Closed (fixed)
Project:
Drupal core
Version:
9.2.x-dev
Component:
update.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
12 Mar 2012 at 08:15 UTC
Updated:
10 Jul 2021 at 18:14 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #9
quietone commentedIs this still valid?
Comment #10
quietone commentedDidn't finish that comment.
I am not one in the know about update tests but surely a lot has change in the 6 years since this issue was opened and this may be outdated. How to prove that?
Comment #11
dwwI'll look into this in the next few days and assess what, if anything, still needs to happen here.
Thanks for flagging this in #BugSmash -- I had completely forgotten about opening this issue. ;)
Cheers,
-Derek
Comment #12
dwwSorry for the delay, this fell off my radar! ;)
Sadly, this isn't outdated. For example:
core/modules/update/tests/fixtures/release-history/bbb_update_test.1_0.xmlcontains:Of particular note:
Part of the trouble would be cleaned up by: #3113798: Remove unused (and generally wrong) <tag> markup from Update module test XML fixtures
That's still lingering in NR, mostly because @tedbow isn't on board, and I haven't convinced him otherwise, nor gotten enough other support for that to do it, anyway. ;)
But some of this is still broken, e.g.
<release_link>vs.<download_link>not agreeing.Not sure if the right solution is to manually/automatically fix things, or remove other attributes from our fixtures that we're clearly not using (or we'd have failing tests).
Comment #13
quietone commentedBased on #12 I made an attempt here. This removes all references to Drupal 7 versions in the fixtures directory. I then went further and modified the release and download links so they were like d.o link.s
Comment #14
quietone commentedDid too much in that one. Starting over.
Comment #15
quietone commented@dww, what do you think about expanding the scope to something like 'clean up the fixtures' and include #2995367: Fix update module test fixture names for 8.2.0-rc2 sample data ?
Comment #16
dww@quietone: Thanks for continuing to work on this. And for the link to #2995367 - never noticed that one before. ;) Seems tightly scoped already. I basically always defer to release managers for issue scoping decisions, and since xjm opened that one as-is, I think we should just fix that independently.
Thanks again,
-Derek
Comment #17
dwwp.s. We've already got #3110917: [meta] Fix update XML fixtures bad data as the generic "update the fixtures" meta. Adding that as the parent for this one.
Comment #18
quietone commented@dww. Thanks. Anything else to do here?
Comment #20
tedbow@quietone and @dww thanks for working on this
I think this 1 is good. Searched under core/modules/update/tests for '7.' and '7-' and only found results in
ModuleVersionTestwhich is intentional.re
I checked this out.
We do assert release links in
\Drupal\Tests\update\Functional\UpdateTestBase::assertVersionUpdateLinks()But for the links that are wrong in the XML:
The projects
bbb_update_testandccc_update_testare only used intestUpdateBrokenFetchURL()andtestUpdateContribOrder()for these tests we don't need to assert the release link.For
update_test_subthemeandupdate_test_subthemewe use them\Drupal\Tests\update\Functional\UpdateContribTest::testUpdateBaseThemeSecurityUpdate()We don't actually assert anything about
update_test_subthemebecause the test is test whether the base theme update will show.We currently have
$this->assertRaw(Link::fromTextAndUrl(t('Update test base theme'), Url::fromUri('http://example.com/project/update_test_basetheme'))->toString());which does assert that there is link to the base theme project page but I think if we changed that to
We would assert the release link is correct and would get the added benefit of asserting the actual version number of the security update.
Comment #21
tedbowUpdated the issue summary to make it easier for committers
Comment #22
quietone commented@tedbow, thanks.
#21. Change from assertRaw to assertVersionUpdateLinks.
Comment #23
tedbow@quietone thanks for the update! Looks good!
Comment #26
xjmCommitted to 9.3.x and cherry-picked to 9.2.x. Thanks!
Comment #27
xjm