Overview

Add a playwright e2e test of the user flow for

1. Creating a content template
2. Confirming that the suggestedPreviewEntityId is used as the example content when the template is open
3. Adding a component to the template
4. Linking a prop of the component instance to a field
5. Publishing the template
6. Checking the published node and that the prop uses the field. Could be good to also check a second node that uses the same content template.
7. Edit the template and add additional component instances to test multiple combinations of complexity on the Field Widget and (Static|Dynamic)PropSource axes: see outline in this MR comment

Another test that would be good to have is testing the flow when a user does not have any content yet. This could potentially be in another issue but noting here so it's not forgotten.

Proposed resolution

Take the approach of having a test be testing one use case versus repeating the same assertions of test setup. Consider creating a test recipe that will do steps 1-3 for the setup? Then have atomically-focused test testing each use case outlined in #7.

User interface changes

Issue fork canvas-3546760

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

hooroomoo created an issue. See original summary.

wim leers’s picture

hooroomoo’s picture

Issue summary: View changes

updated IS to link to correct MR comment (previous link wasn't pointing to the MR comment)

hooroomoo’s picture

Issue summary: View changes
hooroomoo’s picture

Issue summary: View changes
hooroomoo’s picture

Issue summary: View changes

bnjmnm made their first commit to this issue’s fork.

bnjmnm’s picture

Status: Active » Needs review

jptaranto made their first commit to this issue’s fork.

jptaranto’s picture

Ok, I think I rebased this correctly @bnjmnm

wim leers’s picture

Priority: Normal » Critical
Status: Needs review » Needs work
Issue tags: +DX (Developer Experience), +maintainability

@bnjmnm: This MR looks fantastic — I had no idea this was passing all the way back in October! 🤯😱

If so, I'd have merged this as soon as it was green!

@jptaranto: thanks so much for getting this to a mergeable state again! 💙


Looks like we're down to a fairly simple remaining bit?

$ npm run test:playwright
> @drupal-canvas/root@0.0.0 test:playwright
> playwright install && playwright test
Error: Cannot find module './utilities/DrupalFilesystem'
Require stack:
- /builds/project/canvas/_drupal/web/modules/contrib/canvas/tests/src/Playwright/templates-linking.spec.ts
- /builds/project/canvas/_drupal/web/modules/contrib/canvas/node_modules/playwright/lib/transform/transform.js
- /builds/project/canvas/_drupal/web/modules/contrib/canvas/node_modules/playwright/lib/common/configLoader.js
- /builds/project/canvas/_drupal/web/modules/contrib/canvas/node_modules/playwright/lib/program.js
- /builds/project/canvas/_drupal/web/modules/contrib/canvas/node_modules/@playwright/test/cli.js
   at templates-linking.spec.ts:5
  3 | import { test } from './fixtures/DrupalSite';
  4 | import { Drupal } from './objects/Drupal';
> 5 | import { getModuleDir } from './utilities/DrupalFilesystem';
    | ^
  6 |
  7 | // Each object property is a prop type with a value of an array of the available
  8 | // suggestions to it.
    at Object.<anonymous> (/builds/project/canvas/_drupal/web/modules/contrib/canvas/tests/src/Playwright/templates-linking.spec.ts:5:1)

jessebaker made their first commit to this issue’s fork.

jessebaker’s picture

Title: Add e2e test for `ContentTemplates` feature » [PP-1] Add e2e test for `ContentTemplates` feature
Status: Needs work » Postponed

Postponing issue awaiting merge of #3571599: "Template for Basic page not found" error after adding new template which has poached one of the tests originally implemented in this MR and resolved some further issues.

This issue can be returned to once that one has landed.

wim leers’s picture

Title: [PP-1] Add e2e test for `ContentTemplates` feature » Add e2e test for `ContentTemplates` feature
Component: Page builder » Theme builder
Assigned: Unassigned » jessebaker
Status: Postponed » Needs work
jessebaker’s picture

I think there is a bug (well, more specifically a performance issue) that is resulting in this test failing.

When you link a prop on the `all props` SDC locally the PATCH request takes between 1.4 and 2 seconds on the server. Compare that to a Code component with just 1 prop which takes around 100ms.

On CI the test seems to keep failing and as far as I can deduce from the videos I'm seeing of the test playing back the issue is that linking the prop is taking over 8 seconds and that's causing some kind of time out. The test/functionality is working, it's just performing so slowly that the test runner is giving up waiting for it.

bnjmnm’s picture

I think there is a bug (well, more specifically a performance issue) that is resulting in this test failing.

When you link a prop on the `all props` SDC locally the PATCH request takes between 1.4 and 2 seconds on the server. Compare that to a Code component with just 1 prop which takes around 100ms.

I spotted an inefficiency where we were calling for suggestions within a loop multiple times when once before the loop would suffice. I think this is a fairly simple fix but we'll see...

wim leers’s picture

#17++ 👏

Reviewed the MR — some good progress here, some Qs.

bnjmnm’s picture

Assigned: jessebaker » bnjmnm

🤦 although I got this back to passing everything, I realized there are linking options that have been added since this was originally created a few months back. The tests need to reflect those additions AND be able to recognize when such additions occur.

bnjmnm’s picture

Assigned: bnjmnm » Unassigned
Status: Needs work » Needs review

Tests have been updated to fail if new options are available for a given prop type or a linker is present that is not included in the tests, and it's current with 1.x overall.

  • jessebaker committed 03f9abcf on 1.x authored by bnjmnm
    feat: #3546760 Add e2e test for `ContentTemplates` feature
    
    By: wim...
jessebaker’s picture

Status: Needs review » Fixed

Merged! It's critical that these tests are in place ahead of #3562896: Manage props/page data forms with react-hook-form as without them there are large parts of the UI that are not covered that may be impacted by those changes.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.