Closed (fixed)
Project:
Drupal core
Version:
8.3.x-dev
Component:
content_moderation.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
25 Mar 2017 at 16:39 UTC
Updated:
19 Oct 2023 at 08:50 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
nlisgo commentedI have fixed all but one of the failing tests. I believe the failing test is evidence that there is a bug in core. The test passed in simpletest because it was checking for text by doing a case sensitive search:
The above assertion is failing because the raw text "Save and Publish" is found as assertNoRaw is case insensitive with BrowserTestBase.
I will create a separate issue and amend the test so that we can prove it is broken. I will relate that test to this one once it is created.
Comment #4
nlisgo commentedComment #5
nlisgo commentedI'm going to wait for feedback on #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading and if there is a quick resolution there then I will pick this up again and try and push for a complete conversion of simpletests. Otherwise I will remove the \Drupal\content_moderation\Tests\ModerationStateNodeTypeTest conversion so we can get some of the conversions through and then create a followup issue for #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading
Comment #6
nlisgo commentedIt appears that the intent of the test was to do a case sensitive check. See: #2864060-5: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading.
We should supply a patch that performs a case sensitive check for the test "Save and publish"
Comment #7
eric_a commentedComment #8
nlisgo commentedComment #9
timmillwoodLooks good apart from the ModerationStateNodeTypeTest change. I think #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading needs committing first.
Comment #10
nlisgo commentedIt's my view that neither issue need hold up the other. If #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading gets committed first we can reroll and refactor this patch and vice versa.
This patch provides the same level of coverage as is in core currently.
But let's hope #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading gets committed soon and we don't even need to think about it :)
Comment #11
nlisgo commentedDuplicate of last message. Not sure how to delete.
Comment #12
dawehnerI think especially given this patch is mostly about file moving it would probably still apply. Blocking something for the sake of it feels always like an abuse of the system.
Comment #13
timmillwoodOk, let's RTBC this and get it in.
Comment #15
nlisgo commentedComment #17
nlisgo commentedComment #18
nlisgo commentedComment #19
nlisgo commentedNow that #2864060: ModerationStateNodeTypeTest::testEnablingOnExistingContent assertion is misleading has been committed, this patch required a re-roll.
Comment #20
timmillwoodBack to RTBC if testbot agrees.
Comment #22
timmillwoodTest fail looks to be unrelated.
Comment #23
michielnugter commentedComment #25
catchCommitted/pushed to 8.4.x, thanks!
Comment #26
catchAnd cherry-picked to 8.3.x, this caused a problem with #2868429: ModerationStateWidget depends on EntityTypeInterface::getBundleEntityType despite content moderation supporting entity types without a bundle..
Comment #29
nlisgo commented