[MGNLTEST-235] Cleanup fails to delete nodes with UTF8-chars in node-/file-name - failing silently! Created: 20/May/22  Updated: 16/Jun/22  Resolved: 10/Jun/22

Status: Closed
Project: Magnolia Test Framework
Component/s: None
Affects Version/s: 1.3.1
Fix Version/s: 1.3.2

Type: Bug Priority: Critical
Reporter: Christoph Meier Assignee: Michael Duerig
Resolution: Fixed Votes: 0
Labels: foundation_team
Remaining Estimate: Not Specified
Time Spent: 2.5h
Original Estimate: Not Specified

Issue Links:
Problem/Incident
is caused by MGNLREST-394 REST requests with Umlaut characters ... Closed
relation
is related to MGNLTEST-236 MagnoliaRestClient#nodeExists fails t... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[X]* Doc/release notes changes? Comment present?
[X]* Downstream builds green?
[X]* Solution information and context easily available?
[X]* Tests
[X]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[X]* Steps to reproduce, expected, and actual results filled
[X]* Affected version filled
Date of First Response:
Team: Foundation

 Description   

This example from PagesCoreFunctionalTests#pageNodeWithUtf8Chars

fails since 6.3-hackathon/site-removal.

@Cleanup("website:/äëïöü")

 
Fixture usually works. (It may fail in rare cases, but that could be a timing issue).

(a) But Cleanup fails always - if the node name contains UTF8 characters.

(b) And additional problem is that Cleanup fails silently.
Cleanup must throw an exception if it fails!

Related to (b)
(c) Cleanup is called 2 times:

  • @beforeEach
  • @afterEach

See info.magnolia.test.fixture.JcrFixture.
Cleanup should be called only once  - @afterEach. That's actually a must, once (b) is fixed.

 



 Comments   
Comment by Michael Duerig [ 24/May/22 ]

cmeier re.

a) MGNLREST-394 fixes this

b) clean up actually does throw an exception when it fails. With the issue from MGNLREST-394 it just behaved strange as it reported to have succeeded but the node was not deleted after all.

c) not sure whether this is a good idea (at least at this point in time). I have some reminiscence of this being done on purpose. Maybe rdhar or mgeljic remember why we run fixture cleanup also before executing the test method. In any case, I would postpone this to a separate ticket and tackle it once we are done with our effort on fixing the tests that broke after the M6.3 merger.

Comment by Christoph Meier [ 25/May/22 ]

mduerig 

(b)
I will test this again as soon as MGNLREST-394 will be integrated.

(c)
If the node to clean does not exist and the attempt to clean the "nothing" remains  "silent" - fine for me.
But still I'm curious why the Cleanup is called also before. 
Let's see ...

 

Comment by Michael Duerig [ 25/May/22 ]

Re-enabled pageNodeWithUtf8Chars, which was failing because of MGNLREST-394: https://git.magnolia-cms.com/projects/PLATFORM/repos/ce/commits/c950edac4cec5148e793497bc0d8b917d1feec8d

The test passes now for me locally. I keep an eye on Jenkins once the test passes there I resolve this ticket as fixed.

Comment by Christoph Meier [ 06/Jun/22 ]

For the records:

The issue has been fixed via MGNLREST-394 

Cleanup is still called twice.

When called during @beforeEach - and if there is nothing to clean - it remains silent, which is good.

If Cleanup finds the resource but fails to clean it up - it throws an exception.

That's fine. 

Closing this one now

Comment by Christoph Meier [ 07/Jun/22 ]

Adding same comment to both MGNLTEST-236, MGNLTEST-235. Both reopened.

The issues still exist.
What exactly happens?

on release/6.2-branch

File website.ääööüü.yaml can be loaded.
But the assertion

assertTrue(magnoliaRestClient.nodeExists("website", "/ääööüü"));

fails.

AND
@Cleanup("website:/ääööüü")
does not work - silently. (not deleting, but not throwing an exception)

on master:
Unfortunately, exactly the same
and FTR: master contains rest-2.3-SNAPSHOT - which includes the fix on the rest-dsipatcher-servlet

Generated at Mon Feb 12 07:46:55 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.