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
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
Comment #2
wim leersComment #3
hooroomooupdated IS to link to correct MR comment (previous link wasn't pointing to the MR comment)
Comment #4
hooroomooComment #5
hooroomooComment #6
hooroomooComment #9
bnjmnmComment #11
jptarantoOk, I think I rebased this correctly @bnjmnm
Comment #12
wim leers@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?
Comment #14
jessebaker commentedPostponing 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.
Comment #15
wim leers#3571599: "Template for Basic page not found" error after adding new template landed!
Comment #16
jessebaker commentedI 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.
Comment #17
bnjmnmI 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...
Comment #18
wim leers#17++ 👏
Reviewed the MR — some good progress here, some Qs.
Comment #19
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.
Comment #20
bnjmnmTests 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.
Comment #22
jessebaker commentedMerged! 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.