Skip to content
This repository was archived by the owner on Mar 13, 2024. It is now read-only.

Migrated support files and accessibility sidebar spec file to Typescript#10910

Merged
joebigboi merged 9 commits into
masterfrom
Cypress-to-Typescript-Accessibility_sidebar_spec
Aug 29, 2022
Merged

Migrated support files and accessibility sidebar spec file to Typescript#10910
joebigboi merged 9 commits into
masterfrom
Cypress-to-Typescript-Accessibility_sidebar_spec

Conversation

@furqanmlk
Copy link
Copy Markdown
Contributor

Summary

Converted following support files

  1. e2e/cypress/tests/support/ui/sidebar_left.ts
  2. e2e/cypress/tests/support/task_commands.ts
  3. e2e/cypress/tests/plugins/external_request.ts
  4. e2e/cypress/tests/support/api_commands.ts
  5. e2e/cypress/tests/support/env.ts
  6. e2e/cypress/tests/support/task_commands.ts
  7. e2e/cypress/tests/support/ui/post.ts
  8. e2e/cypress/tests/support/ui/sidebar_left.ts

and one spec file accessibility_sidebar_spec.ts

Ticket Link

https://mattermost.atlassian.net/browse/MM-45968

Screenshots

image

Release Note

NONE

@furqanmlk
Copy link
Copy Markdown
Contributor Author

/e2e-test

@mattermod
Copy link
Copy Markdown
Contributor

@saturninoabril
Copy link
Copy Markdown
Contributor

/e2e-test

@mattermod
Copy link
Copy Markdown
Contributor

@furqanmlk furqanmlk changed the title converted support files and accessibility sidebar spec file Migrated support files and accessibility sidebar spec file to Typescript Aug 11, 2022
Furqan Malik added 2 commits August 16, 2022 15:01
…thub.com:mattermost/mattermost-webapp into Cypress-to-Typescript-Accessibility_sidebar_spec
@furqanmlk
Copy link
Copy Markdown
Contributor Author

/e2e-test

@mattermod
Copy link
Copy Markdown
Contributor

@saturninoabril saturninoabril requested review from mvitale1989 and removed request for saturninoabril August 17, 2022 07:21
Copy link
Copy Markdown
Contributor

@neallred neallred left a comment

Choose a reason for hiding this comment

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

Nice work! Cool to see the tests still passing. It looks like we can remove some of the declaration files for files that were migrated: e2e/cypress/tests/support/task_commands.d.ts, e2e/cypress/tests/support/ui_commands.d.ts, e2e/cypress/tests/support/ui/post.d.ts.

Could you make sure that the npm run check-types command completes successfully when run locally from the e2e/cypress directory? I'm getting a number of errors when I run it locally. I should have added running that step to the CI pipelines in the PR that introduced Typescript support, but missed it. We should probably add a ticket to do that as part of the e2e typescript migration; it will avoid conflicts and confusion if we make sure that each change type checks automatically.

Comment thread e2e/cypress/tests/support/api_commands.ts Outdated
Comment thread e2e/cypress/tests/support/api_commands.ts Outdated
Comment thread e2e/cypress/tests/support/api_commands.ts Outdated
Comment thread e2e/cypress/tests/support/api_commands.ts Outdated
Comment thread e2e/cypress/tests/support/api_commands.ts
Comment thread e2e/cypress/tests/support/task_commands.ts Outdated
Comment thread e2e/cypress/tests/integration/accessibility/accessibility_sidebar_spec.ts Outdated
Copy link
Copy Markdown
Contributor

@joebigboi joebigboi left a comment

Choose a reason for hiding this comment

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

LGTM. Just one minor query

Comment on lines 35 to +36
const setCookie = response.headers['set-cookie'];
setCookie.forEach((cookie) => {
(setCookie as any).forEach((cookie: string) => {
Copy link
Copy Markdown
Contributor

@joebigboi joebigboi Aug 17, 2022

Choose a reason for hiding this comment

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

shouldn't this be const setCookie: string[] = response.headers['set-cookie']; and then you can write setCookie.forEach((cookie) => {

Copy link
Copy Markdown
Contributor

@mvitale1989 mvitale1989 left a comment

Choose a reason for hiding this comment

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

Tests are passing, and afaict this looks good to me

@furqanmlk furqanmlk requested a review from neallred August 24, 2022 14:55
@furqanmlk
Copy link
Copy Markdown
Contributor Author

@neallred
I will push your recommended changes, shortly.
Sorry I just saw your message

@furqanmlk furqanmlk requested a review from a team as a code owner August 25, 2022 20:48
@furqanmlk furqanmlk force-pushed the Cypress-to-Typescript-Accessibility_sidebar_spec branch from 46b2415 to 4389091 Compare August 25, 2022 21:09
Copy link
Copy Markdown
Contributor

@neallred neallred left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @furqanmlk !

@furqanmlk
Copy link
Copy Markdown
Contributor Author

/e2e-test

This was referenced Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants