-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add end 2 end test tag creation #13129
Conversation
test/e2e/specs/taxonomies.test.js
Outdated
@@ -36,6 +36,19 @@ describe( 'Taxonomies', () => { | |||
); | |||
}; | |||
|
|||
const getCurrentTags = async () => { | |||
const tagsPanel = await findSidebarPanelWithTitle( 'Tags' ); | |||
return tagsPanel.$eval( '*', ( node ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use a *
selector. If we're operating on the panel, couldn't we go direct to selecting the tokens?
tagsPanel.$$eval( '.components-form-token-field__token-text span:not(.screen-reader-text)', ( node ) => {
Or is tagsPanel
not the panel (looking at the implementation of findSidebarPanelWithTitle
)? If so, what sense does that make?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aduth, findSidebarPanelWithTitle does not return the main panel content that we need here. findSidebarPanelWithTitle returns a button inside the panel that allows toggling the panel.
Here I want to query '.components-form-token-field__token-text span:not(.screen-reader-text)' inside the main tag panel.
The main tag panel is the "grandparent" of the toggle button. To the best way to evaluate something on the grandparent of an element seemed to be doing tagsPanel.$eval( '*'
that will evaluate something on a node child of the element and then get the grandgrandparent of that node.
This seems like a hack I think findSidebarPanelWithTitle should probably return the main container of the panel and not the toggle button of the panel. Given that in most cases we just want the toggle button maybe we should have another util findSidebarPanelToggleButtonWithTitle that return the toggle button and will be used in most cases that currently use findSidebarPanelWithTitle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @aduth I applied the referred change. We now have two utils:
-findSidebarPanelToggleButtonWithTitle: Returns the button that when clicked toggles the sidebar panel open state. The previous findSidebarPanelWithTitle.
-findSidebarPanelWithTitle: Returns the sidebar panel itself, so test developers can then query things inside the panel. Before as the button was returned to query things we would need hacks e.g: parent.parent...
db97579
to
8f5a7b7
Compare
21de46b
to
3009101
Compare
7636460
to
6b8eeb3
Compare
6b8eeb3
to
88d5d33
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new test looks good and changes applied to the existing one make sense.
I’m wondering how we can ensure that the tests ever reach the part after permission check. It seems like it was dead code before.
Thank you for the reviews!
Before we had a bug on category vs categories so yes we had a dead code problem that was fixed here. |
I think this new test is a source of intermittent failures, even after some more recent improvements (#14219). At the moment there's a failure on #14276 which is a totally unrelated change:
|
Description
This PR just adds an end 2 end test that tests the tag creation. It is the continuation of a process where we want to cover taxonomies with the end 2 end tests to avoid future regressions.
More taxonomy related tests will follow.
How has this been tested?
Verify the end 2 end tests pass.