Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Fix editor history link test #50849

Closed

Conversation

dcalhoun
Copy link
Member

What?

Repair the failing editor history link test that logged errors.

Why?

Cover regression-prone features with automated tests.

How?

test: Fix React prop typo
The testID typo was the root cause for test failures.

test: Simplify test queries
The targeted elements do not require asynchronous queries. Using
synchronous queries is more concise and easier to comprehend.

test: Increase editor history link test accuracy
The targeted bug occurs when:

  1. Changing a link to "Open in a new tab."
  2. Performing additional changes, e.g. typing.
  3. Undoing both actions.
  4. Redoing both actions.

Thus, this changes the initial HTML, adds additional undo/redo
invocations, and updates inline snapshots to match the expected
outcomes.

Testing Instructions

n/a

Testing Instructions for Keyboard

n/a

Screenshots or screencast

n/a

Mamaduka and others added 30 commits May 11, 2023 09:16
…ks component (#49161)

* Navigation: Remove the check for draft navigation menus from the UnsavedInnerBlocks component

* Add a new selector to check the status of all navigation menus
…48683)

* Scaffold out preloading

* All GET and OPTIONS preloading

* Make preload paths more readable via add_query_arg

* Add comment to unusual usage

* Resolve PHPCS

* Fix formatting

* Rename away from “permissions”

* Limit to Site Editor

* Add context to doc block

* Preload Browse Mode sidebar Navigation

* Remove redundant preload

* Use int not string for numeric
* WIP - lang attribute

* applying attributes

* Porting of JB plugin.

* lang & dir are defined in the upper scope

* add translation icon

* rename to language

* migrate to useAnchor

* Use the title

* rename function to Edit

* Strings tweaks

* Fix styles

* a11y tweak - add aria-describedby to the paragraph

* Use "help" instead of a paragraph

* Remove aria-describedby

* Add missing role="menuitemcheckbox"

* Update packages/format-library/src/language/index.js

Co-authored-by: George Mamadashvili <[email protected]>

* Update packages/format-library/src/language/index.js

Co-authored-by: George Mamadashvili <[email protected]>

* move the language format definition before the component

* change anchorRef to popoverAnchor

* Revert "move the language format definition before the component"

This reverts commit 560fcbb.

* Move language above Edit

* change class to has-language

* Use a form

* change classnames for the form and the popover

* Update packages/format-library/src/language/index.js

Co-authored-by: George Mamadashvili <[email protected]>

* Update packages/format-library/src/language/index.js

Co-authored-by: George Mamadashvili <[email protected]>

* CS fix

* Add event.preventDefault()

* Update icon

* Use a dropdown & automate RTL

* Revert "Use a dropdown & automate RTL"

This reverts commit df5bb36.

* rename icon from "translation" to "language"

* right-align the button

---------

Co-authored-by: Andrea Fercia <[email protected]>
Co-authored-by: George Mamadashvili <[email protected]>
* Plugin: Use bundled files for enqueuing global styles

* Override the '__experimentalFeatures'

* Recreate global styles for block editors

* Move global styles custom CSS handlers

* Move _gutenberg_clean_theme_json_caches

* Override global styles custom CSS properties
…ar navigation (#50489)

* Move LeafMoreMenu into list-view so OffCanvasEditor can be eventually be deleted

Move LeafMoreMenu to its own component

Move Leaf More Menu to navigation block

remove private API

Add Leaf More Menu to the browse mode sidebar

Add a note to show these files are duplicated

* Change block title

* minor change

---------

Co-authored-by: scruffian <[email protected]>
* Release script: Update react-native-editor version to 1.95.0

* Release script: Update with changes from 'npm run core preios'

* Update react-native-editor changelog
… requests (#49839)

* Adjust the logic of Post Title edit, so it avoids unnecessary OPTIONS requests

* Fix lint issues

* Update the change comment description

* Refactor useCanEditEntity arguments in post-title following  code review suggestion
* Mobile - Update E2E config to support an iPad simulator

* Mobile - E2E utils - Update code to not add an extra object spread
* test: Await overlooked asynchronous task

The test helper executes asynchronous updates to the layout. If this is
not awaited, test failures can occur in certain circumstances.

* feat: Combine BlockList and BlockListCompact

Reduce code duplication between the separate components, e.g. item
render, footer, end-of-list block appender button, empty list
placeholder.

* wip: Gallery, Columns working; Buttons broken

Partially fix multi-column layouts by passing existing styles to a
wrapping view. All of the passed styles may not be necessary, need to
investigate. Also, Buttons render broken, wrapping lines and overflowing
the parent container.

* test: Fix nested lists tests

Nested lists now rely upon `BlockListItem`, which returns `null` if the
`blockWidth` has not yet been set. In order for test queries for nested
list items to succeed, we must trigger the `onLayout` callback for the
nested block lists. `BlockListItem` is now used to expand capabilities
for nested blocks, e.g. multi-column grid items.

* wip: Fix Blocks button layout

Fixes Buttons, but Columns remain broken.

* refactor: Remove unused BlockList title prop

* refactor: Separate inner block list styles

The list is no longer shared, so we can merely set the appropriate
styles on each list element.

* refactor: Remove unused virtualized listKey

Now that inner blocks do not use a virtualized list, a unique list key
is no longer necessary.

* refactor: Remove outdated inline comment

The scroll ref is no longer passed to inner blocks as they do not use
virtualized lists.

* wip: Explore nest Columns fixes

The relocated styles may need to be separate from the top-level block
list element, as the styles may conflict with other styles. It does not
fix Columns, however.

* Mobile - BlockList - Fix layout issues for stacked horizontally blocks

* fix: Revert Buttons alignment workaround

This caused issues for non-Buttons inner block alignment. The original
issue of Buttons inner blocks not rendering inline was addressed in
43c0b14918f8ed03037c01d94321922dc31a7fa3.

* fix: Column width sums less than 100% correct align

These styles caused columns to align to the center when their width sum
did not equal or execeed 100%. Removing these styles did not appear to
negatively impact other inner blocks or the use cases outlined in
#25621.

* refactor: Inner blocks use clientId key

This mirrors the approach for cells of top-level block lists. It is also
likely more robust, providing better performance when reordering blocks.

* refactor: Remove unnecessary key

The inner blocks list now sets a key on a wrapping view, rather than via
the `renderItem` function. Thus, this key is no superfluous.

* fix: Prevent block list footer from stretching to parent height

The parent flex styles were applied to the footer element. This wrapping
view prevents those styles from erroneously stretching the provided
footer element.

* Mobile - List block - Remove usage of useCompactList and disables the option to render the appender for InnerBlocks

* test: Update Android e2e Buttons block Xpath selector

This Xpath selector became outdated with the block list refactor.

* test: Delay query for block actions menu

Appium reported this cached element had been removed from the DOM.
Relocating the query seemingly resolved this issue.

---------

Co-authored-by: Gerardo <[email protected]>
… `ReadableContentView` (#50552)

* Mobile - BlockList - EmptyList: Use useEditorWrapperStyles hook instead of ReadableContentView

* Mobile - Remove ReadableContentView component
* Lodash: Remove from template part block

* Fix logic
- adds an `_edit_link` property to `wp_global_styles`, `wp_template`, and `wp_template-part` custom post type schemata via filter
- uses the `_edit_link` value to create an edit link via the `get_edit_post_link` hook
@derekblank
Copy link
Member

derekblank commented May 23, 2023

These changes result in a passing test without errors; the PR description and individual commits explain the nuance changes to achieve this.

@dcalhoun Thank you very much for taking a look and applying the fixes. 🙇

Unfortunately, I do not believe it accurately tests the spirit of the bug fix: editor history actions after redoing changes to a link's target are not lost.

I agree that the test does not fully capture the full bug behavior on its own due to limitations with integration test capability, and would likely be better captured with an E2E test.

If you revert the fix itself, you can observe that the test fails due to the relocation of the rel property. This is expected, however, I also anticipated the second sentence to be missing entirely in the final snapshot, i.e. the "lost" editor history.

Precisely -- within the app, the relocation of the rel attribute appears to be the cause that fires the extra state event, as mentioned here. The mechanics of this extra state event are not yet understood without deeper investigation, but the root cause is possibly within Aztec, as you mentioned. The fix itself does appear to rectify the issue for the end user. While the integration test can capture that the rel attribute does not shift, the observation that the "lost" editor history is not lost in the integration test reinforces that an E2E test would be better suited.

@derekblank
Copy link
Member

The failing Unit Tests task appears to be due to the log of a warning message: Browserslist: caniuse-lite is outdated. This also occurs in test/fix-editor-history-link-test, but not in the latest trunk branch. So, I anticipate this failure will be resolved once the latest from trunk is merged into test/fix-editor-history-link-test.

I merged the latest from trunk into this branch. I believe it caused the commit history mess due to not having merged the latest from trunk into the target branch, rnmobile/fix/undo-redo-history -- apologies. I think cherry-picking the three original commits you added to the target branch and closing this PR may be the best approach, and then the discussion/work for the remaining E2E testing approach and utility of the integration test can continue on the target PR. WDYT?

@derekblank derekblank closed this May 23, 2023
@derekblank derekblank deleted the test/fix-editor-history-link-test branch May 23, 2023 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.