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

[Upgrade Assistant] Fix cits, a11y and functional tests #118778

Conversation

sabarasaba
Copy link
Member

@sabarasaba sabarasaba commented Nov 16, 2021

This PR fixes the tests (CITs, functional and a11y) for the changes that got introduced in #118659

@sabarasaba sabarasaba requested a review from sebelga November 17, 2021 09:13
@sabarasaba sabarasaba self-assigned this Nov 17, 2021
@sabarasaba sabarasaba marked this pull request as ready for review November 17, 2021 09:13
Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @sabarasaba ! Overall looks good to me. I left a few comments around naming and organising tests (I think we can move all CIT in the same file for the es deprecation logs).

I left a comment for a change I didn't understand where we can interchangeably use different request mock with the same result.

await PageObjects.upgradeAssistant.navigateToFixDeprecationLogs();

// Only click deprecation logging toggle if its not already enabled
if (!(await testSubjects.isDisplayed('externalLinksTitle'))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

externalLinksTitle does not seem to reference the actual DOM node. It seems to be the title of the page "Analyze deprecation logs". Can we update it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh re-reading this if block, i think it can be safely deleted since deprecation toggle is enabled by default now. This was used before when we had to keep it enabled for all tests.

await PageObjects.upgradeAssistant.clickDeprecationLoggingToggle();
}

await retry.waitFor('UA external links title to be present', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean "external links"? I might miss the context.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

External links is the name of the component that shows links to the discovery and observability apps. I think this check can be removed now (see above).

@kibana-ci
Copy link
Collaborator

💔 Build Failed

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [384f5f2]

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @sabarasaba

@sabarasaba sabarasaba merged commit 1121331 into elastic:ua/deprecation_logs_improvements Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants