Write end-to-end test for dragging components into slots

Created on 11 September 2024, 7 months ago

Overview

The codebase has an E2E spec named component-slots which was introduced to test dragging components into slots. E.g. dragging an Image component inside one of the columns of a Two Column component. To aid this, the realDnD() command was introduced (tests/src/Cypress/cypress/support/realDnd.js).

Making this test work reliably became increasingly difficult after πŸ› Allow dragging components to top/bottom of page and in between adjacent components with slots Fixed . We couldn't simply target the example content of a slot anymore, we needed to make sure that the slot is detected instead of a potential drop target adjacent to the component that provides the slot.

After trying to improve the realDnD command to support this, we decided that in favor of productivity, we disable running the end-to-end test. This is currently done by using describe.skip() in the E2E spec file.

Proposed resolution

  1. Remove the use of describe.skip() in components-slots.cy.js#L1.
  2. Come up with a solid way of dropping a component into a slot in an end-to-end test.

Approaches tried so far β€” adjustments to the realDnD command:

  • Slowing down the movement of the mouse cursor;
  • Supporting customizable segments of the movement with variable speed;
  • Supporting intermediate stops with custom durations;

A successful strategy could be that we create components specifically for these tests where empty slots have oversized drop targets. Maybe that in combination with approaches already tried can work.

πŸ“Œ Task
Status

Postponed

Component

Page builder

Created by

πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @balintbrews
  • πŸ‡³πŸ‡±Netherlands balintbrews Amsterdam, NL
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    We still might not want to do this right now but I think we would not have hit πŸ› Unable to add SDCs with slots to the page Active if we had the test.

    It seems like "slots" is a very basic functionality for XB and we don't know if any commit will break that should be consider "major" priority to fix it.
    could we at least add back the part of the test that drags the "two column" element on?

  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    I know this issue was about testing for dragging components into slots but since we skip components-slots.cy.js completely doesn't that mean we also don't have e2e coverage for dragging a component with slots on the canvas? Could we do just the part of the test where we add the "Two column" component on to the canvas and then fix the rest later?

    I think we would not have had the regression in πŸ› Unable to add SDCs with slots to the page Active if we at least had that.

    If seems like from the issue summary that problem is testing placing items in the slot not placing an item with slots the canvas, correct?

    I think we should add test coverage soon for what we can

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    @tedbow++ β€” bumping priority.

  • πŸ‡ΊπŸ‡ΈUnited States hooroomoo
  • πŸ‡¬πŸ‡§United Kingdom jessebaker

    I don't think is particularly valuable to have an end to end test that ACTUALLY drags and drops elements (e.g. mousedown, mousemove, mouseover, mouseup etc) . It is a lot of work, is prone to failure etc. And we use a 3rd party library that I assume has its own tests anyway. Furthermore it's immediately obvious to any human that it is broken as it's such a core piece of the XB functionality so it's unlikely to get missed for any length of time.

    Instead, I wonder if we can find a way to programmatically call the same reducer function that occurs on the drop event directly from Cypress. This would ensure the same code runs (updating the JSON, sending the JSON to the server, receiving the updated HTML, updating the preview) without having to actually do all the fiddly stuff of emulating mouse moves and hover events etc.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Based on a retrospective discussion we just had: +1 to #7. I did not realize in #5 that it was so onerous to write this, but upon re-reading the title ("dragging […]"), it is obvious! 😬

    so it's unlikely to get missed for any length of time.

    … which is also true here: πŸ› Unable to add SDCs with slots to the page Active was reported on Jan 8, 15:49 CET, and it was introduced in πŸ“Œ Throw exceptions instead of returning tuples for validation errors Active on Jan 7, 12:14 CET. Within 27.5 hours!

    Instead, I wonder if we can find a way to programmatically call the same reducer function that occurs on the drop event directly from Cypress. This would ensure the same code runs (updating the JSON, sending the JSON to the server, receiving the updated HTML, updating the preview) without having to actually do all the fiddly stuff of emulating mouse moves and hover events etc.

    So, a unit/kernel-esque test? If so: sounds perfect!

  • πŸ‡ΊπŸ‡ΈUnited States hooroomoo

    I am un-assigning myself from this issue for now and going to add an e2e test that is suggested in #4 in a separate issue πŸ“Œ Add e2e test that adds a component with an empty slot to the canvas Active where I am going to add the component not with dragging but by using the "Add component" click to insert mechanism because adding that test should be easy and I agree with Ted would be worth having.

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    πŸ‘πŸ₯³

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Jesse’s call on where to take this after #7 :)

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA

    #7

    I don't think is particularly valuable to have an end to end test that ACTUALLY drags and drops elements (e.g. mousedown, 
    mousemove, mouseover, mouseup etc) . It is a lot of work, is prone to failure etc

    @jessebaker and/or @hooroomoo are the same actions possible through keyboard actions. If so would this be easier to test?

  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡¬πŸ‡§United Kingdom jessebaker

    RE: #14

    I think this should be postponed until either

Production build 0.71.5 2024