-
Notifications
You must be signed in to change notification settings - Fork 18
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
Setup CZI Playwright framework #806
Conversation
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.
thank you for this!! looking good overall, left a few comments 🙏
import { expect, test } from '@playwright/test'; | ||
import { satisfies } from '@renovate/pep440'; | ||
|
||
import { testPluginFilter } from '../../utils/filter'; |
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.
nit: can we change all the relative imports to absolute imports please? so it should be:
import { testPluginFilter } from '@/e2e/utils/filter';
relative imports within the same parent is fine though:
import { testPluginFilter } from './filter';
import { testPluginFilter } from './utils/filter';
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.
Playwright does not find imports this way and there have been extensive discussions on this. Will need to investigate further. I will suggest we fix this in a separate PR
@charles-testco Is this PR still actively being worked on, or are yours and Victor's other branches replacing this? If this is still the main PR to get the framework integrated, just wanted to make sure there was a plan to address the feedback here. Thank you! |
Hi @richaagarwal this PR implements the bare framework and should go first. We will have few others that will follow today tomorrow but this is the base and needs to go first. I am looking at the feedbacks and hope to address all major ones by today. |
@codemonkey800 should I merge this PR now? |
No description provided.