Skip to content

Conversation

@Eason09053360
Copy link

Add E2E tests for Plugins page (/plugins) to verify plugins list display and pagination functionality.

This PR implements:

  • PluginsPage page object following the Page Object Model pattern
  • Test specs covering plugins list display and pagination
  • Tests passing across Chromium, Firefox, and WebKit browsers (24/24 tests passing)
截圖 2026-01-26 下午2 46 20

Closes: #60571


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: GitHub Copilot following the guidelines

@boring-cyborg
Copy link

boring-cyborg bot commented Jan 26, 2026

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)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Jan 26, 2026
@Eason09053360 Eason09053360 marked this pull request as ready for review January 26, 2026 07:42
test.describe("Plugins Pagination", () => {
let pluginsPage: PluginsPage;

test.beforeEach(async ({ page }) => {
Copy link
Contributor

@henry3260 henry3260 Jan 26, 2026

Choose a reason for hiding this comment

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

I think you have to remove async to fix CI

Copy link
Author

Choose a reason for hiding this comment

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

Applied! Thanks

- Create PluginsPage page object following POM pattern
- Add test specs for plugins list display and pagination
- Tests work across Chromium, Firefox, and WebKit browsers
- All 24 tests passing

Fixes apache#60571
const pagination = pluginsPage.page.locator('[data-scope="pagination"]');
const paginationExists = await pagination.isVisible().catch(() => false);

if (paginationExists) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Either ensure enough data exists in the test environment, or fail explicitly if pagination isn't available

Copy link
Contributor

Choose a reason for hiding this comment

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

await expect(nextButton).toBeVisible(); await expect(nextButton).toBeEnabled();


if (paginationExists) {
// Check if page 2 button exists and is enabled
const page2Button = pagination.getByRole("button", { name: /page 2/i });
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have already defined page.locator('[data-testid="next"]'); we should use that right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider consolidating to one pagination test using next/prev buttons to align with established patterns.

@vatsrahul1001
Copy link
Contributor

Thanks for this PR @Eason09053360 I have added some review comment.
You have two pagination tests that test similar functionality. Consider keeping one test that uses the next/prev buttons (which aligns with our pattern in dags-list.spec.ts). Please check how its done in existing tests

@Eason09053360
Copy link
Author

Thanks for reviewing !You're absolutely right. I've updated the tests to follow the established patterns from dags-list.spec.ts:

Changes :

  • Consolidated two pagination tests into one comprehensive test
  • Using predefined paginationNextButton/paginationPrevButton locators instead of creating temporary ones
  • Removed redundant toBeEnabled() checks

The test now explicitly fails if pagination isn't available, rather than silently passing.

@vatsrahul1001
Copy link
Contributor

Thanks for reviewing !You're absolutely right. I've updated the tests to follow the established patterns from dags-list.spec.ts:

Changes :

  • Consolidated two pagination tests into one comprehensive test
  • Using predefined paginationNextButton/paginationPrevButton locators instead of creating temporary ones
  • Removed redundant toBeEnabled() checks

The test now explicitly fails if pagination isn't available, rather than silently passing.

@Eason09053360 can you look at failing tests

… currently fails in CI due to test environment having only 7 plugins,

which is insufficient to trigger pagination UI controls. Seeking guidance on
whether to keep failing test or remove it.
@Eason09053360
Copy link
Author

I've implemented (fail explicitly) as shown in the current code. The test will fail with a clear error message when pagination buttons are not visible.

I think there are some problems here.

1.Tests are explicit about the limitation (environment has only 7 built-in plugins)
2.CI will always show 3 failed tests (chromium, firefox, webkit) for pagination

Should I keep the failing pagination test to highlight the environment limitation?
Should I remove the pagination test since:
The environment cannot guarantee sufficient plugins
Is there a way to increase plugins in the test environment that I'm not aware of?

@vatsrahul1001
Copy link
Contributor

I see @vishakha1411 has fixed pagination for plugins in PR

@Eason09053360
Copy link
Author

Thank you for pointing me to PR #61059. I've reviewed the fix and understand that Plugins.tsx now correctly implements pagination support

However, the pagination E2E test still fails in the current test environment.
The test environment only has 7 built-in plugins:
截圖 2026-01-28 下午3 27 45

I tested with limit=5&offset=0 to trigger pagination:

Expected: 2 pages (5 plugins on page 1, 2 on page 2) → pagination controls should appear
Actual: Pagination controls are not visible in the UI
Root cause: The DataTable component hides pagination controls when conditions in hasPagination are not met:
const hasPagination = initialState?.pagination !== undefined && (pageIndex !== 0 || rows.length !== total);

When navigating to /plugins?limit=5&offset=0, the URL parameters seem to get cleared during React initialization, resulting in all 7 plugins being displayed on a single page without pagination controls.

@vatsrahul1001
Copy link
Contributor

limit=5&offset=0

@Eason09053360 I suggest you to check how we are handling this in eventpage test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UI E2E Test || ADMIN-005: Verify Plugins Page functionality

3 participants