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.

Sign up for GitHub

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

Jump to bottom

Test/e2e phase 1 #450

Merged
merged 20 commits into from
Mar 1, 2024
Merged

Test/e2e phase 1 #450

merged 20 commits into from
Mar 1, 2024

Conversation

devceline
Copy link
Collaborator

@devceline devceline commented Feb 27, 2024 •
edited
Loading

Description

  • Round 1 Of Adding E2E tests
  • With this, the following is tested
    • Subscribe ✔️
    • Unsubscribe ✔️
    • Update preferences ✔️
    • Subscribe and Unsubscribe from multiple dapps and ensure state works ✔️
    • Receive messages ✔️
  • Disable webkit tests since all actually useful tests were disabled on Webklit and it was causing nothing but headaches

Type of change

  • Tests
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Draft PR (breaking/non-breaking change which needs more work for having a proper functionality [Mark this PR as ready to review only when completely ready])
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Note

The following is yet to be tested and could be very beneficial to test:

  • Paging of notification history ❌
  • Further testing of preferences ❌
  • UI tests / responsiveness ❌
  • Read more in messages ❌

@vercel Vercel
Copy link

vercel bot commented Feb 27, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
app-web3inbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 29, 2024 11:42am

bkrem
bkrem reviewed Feb 27, 2024
View reviewed changes
tests/shared/pages/InboxPage.ts Outdated Show resolved Hide resolved
.github/workflows/ui_test.yml Outdated Show resolved Hide resolved
tests/shared/constants/index.ts Show resolved Hide resolved
tests/shared/helpers/notifyServer.ts Outdated Show resolved Hide resolved
tests/shared/helpers/notifyServer.ts Outdated Show resolved Hide resolved
tests/shared/helpers/notifyServer.ts Outdated Show resolved Hide resolved
tests/subscribe.spec.ts Outdated Show resolved Hide resolved
devceline and others added 2 commits February 27, 2024 23:23
@devceline devceline marked this pull request as ready for review February 28, 2024 11:56
@bkrem
Copy link
Member

bkrem commented Feb 28, 2024

Disable webkit tests since all actually useful tests were disabled on Webklit and it was causing nothing but headaches

What were the issues with webkit? Do we have a plan on how to get webkit back in the game/remove this blindspot?

@devceline
Copy link
Collaborator Author

Disable webkit tests since all actually useful tests were disabled on Webklit and it was causing nothing but headaches

What were the issues with webkit? Do we have a plan on how to get webkit back in the game/remove this blindspot?

There's an issue with loading the wallet page, I didn't investigate too much as this wasn't a critical blocker. I figured I'd create an issue for it and tackle it when we have time to revisit e2e

bkrem
bkrem approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@bkrem bkrem left a comment

Choose a reason for hiding this comment

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

LGTM, great to see this up and running 💪 💯

enesozturk
enesozturk reviewed Feb 28, 2024
View reviewed changes
tests/shared/fixtures/fixture.ts
@@ -1,24 +1,38 @@
import { test as base } from '@playwright/test'

import { ModalPage } from '../pages/InboxPage'
import { ModalValidator } from '../validators/ModalValidator'
import { InboxPage } from '../pages/InboxPage'
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add a tests folder to absolute paths too. (But it might cause some issues on Jest, not really sure, might worth checking)

@devceline devceline merged commit 441b7a8 into main Mar 1, 2024
3 checks passed
@devceline devceline deleted the test/e2e-phase-1 branch March 1, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enesozturk enesozturk enesozturk approved these changes

@bkrem bkrem bkrem approved these changes

@chris13524 chris13524 Awaiting requested review from chris13524

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@devceline @bkrem @chris13524 @enesozturk