-
Notifications
You must be signed in to change notification settings - Fork 16.4k
feat e2e tests to verify for tasks tab functionality #59943
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
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
I tried to make use of |
|
Thanks for the PR @RavinduWeerakoon I will review soon. |
|
@RavinduWeerakoon Thanks for the PR! Good structure and use of data-testid. Here's feedback:
I have also added these in review comments as well please check those as well |
|
@RavinduWeerakoon can you resolve conflicts? |
|
Hi @vatsrahul1001, thanks for the comments. I’ve resolved some of the issues. |
For this PR, I suggest |
|
@RavinduWeerakoon you also needs to rebase |
Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
a88470e to
066b9ac
Compare
Ok, I will remove the meaningless filter tests then |
pierrejeambrun
left a comment
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.
@RavinduWeerakoon Thanks for your work on this PR.
+1 for not making this overly complex by testing every single filters. If the default test dag do not cover those just remove them.
Can you also address the remaining comments, most of them are low effort and this should make this mergeable :)
4ca443b to
9202c6c
Compare
9202c6c to
a1c6b29
Compare
|
Hi @vatsrahul1001, Any other things that I have to do to make this mergable |
Merged. Great work @RavinduWeerakoon ! |
… PR pattern - Remove LoginPage import and all login-related code - Tests now receive authenticated page from Playwright's global storageState - Initialize DagsPage directly in each test function - Matches pattern from successfully merged PRs: apache#59943, apache#59919, apache#59734 - Fixes CI failures across Chromium, Firefox, and WebKit Addresses reviewer feedback from @vatsrahul1001
feat e2e tests to verify for tasks tab functionality Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com> Co-authored-by: vatsrahul1001 <rah.sharma11@gmail.com>
E2E tests to verify the Tasks tab functionality on the DAG detail page
Add 'dag-tasks.spec.ts', which verifies
closes: #59543
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.