Skip to content

Conversation

@haseebmalik18
Copy link

Add E2E tests for DAG audit log functionality

This PR implements comprehensive end-to-end tests for the DAG audit log feature to ensure proper functionality of the Events tab.

Changes

New Files

  • airflow-core/src/airflow/ui/tests/e2e/pages/EventsPage.ts - Page Object Model for the Events/audit log page
  • airflow-core/src/airflow/ui/tests/e2e/specs/dag-audit-log.spec.ts - E2E test specifications

Test Coverage

The implementation includes 5 test scenarios covering:

  1. Navigation and Table Visibility - Verifies users can navigate to the audit log tab and the table renders correctly
  2. Column Verification - Ensures all expected columns (When, Event, User, Extra) are displayed and DAG ID column is hidden in DAG-specific context
  3. Data Validation - Validates that audit log entries contain valid data with proper null/edge case handling
  4. Pagination - Tests navigation between pages and verifies content changes appropriately
  5. Sorting - Validates column header sorting functionality

Implementation Details

  • Follows existing Page Object Model patterns from DagsPage.ts
  • Uses data-testid selectors for stable test automation
  • Includes proper error handling and edge case coverage (empty data, no pagination, insufficient rows)
  • All code passes ESLint with strict alphabetical sorting requirements (perfectionist/sort-classes)
  • Follows TypeScript strict mode conventions

closes: #59684


^ Add meaningful description above
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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg
Copy link

boring-cyborg bot commented Dec 23, 2025

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 Dec 23, 2025
@vatsrahul1001
Copy link
Contributor

@haseebmalik18 can you look at failures?

@haseebmalik18
Copy link
Author

Yes, I've taken a look I understand what the issue is I will have it in a day or two

@haseebmalik18
Copy link
Author

@vatsrahul1001 Just made the fixes. Should be good now.

const sortButton = columnHeader.locator('button[aria-label="sort"]');

await sortButton.click();
await this.page.waitForTimeout(2000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of hardcoding wait. We can wait for specific condition (table reload, URL change) instead.

const setupDagsPage = new DagsPage(page);
const setupEventsPage = new EventsPage(page);

for (let i = 0; i < 10; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Very slow (could take 70+ minutes with 7-minute timeout per trigger)
If one trigger fails, all tests fail
We can Reduce to 2-3 triggers


return true;
} catch {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Methods like isAuditLogTableVisible(), hasNextPage(), getEventLogCount() catch errors and return false This makes debugging difficult and can cause tests to pass when they shouldn't.


const hasNext = await eventsPage.hasNextPage();

if (hasNext) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If pagination isn't available, test passes without testing anything.

Copy link
Author

@haseebmalik18 haseebmalik18 Dec 28, 2025

Choose a reason for hiding this comment

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

Hi @vatsrahul1001 , I've addressed all your feedback. But, I'm not sure how to approach the pagination test without increasing triggers. The problem is that 3 triggers only creates 4 audit log events, but I need 51+ events to test pagination since production default is page_size=50. I'm not quite sure how to approach this as increasing the trigger will be quite slow. Any suggestions?

const allTextContents = await cells.allTextContents();
const hasContent = allTextContents.some((text) => text.trim().length > 0);

expect(hasContent).toBe(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checks "something exists" - doesn't verify actual event data like event type, timestamp format, etc.


const orderChanged = JSON.stringify(initialEvents) !== JSON.stringify(sortedEvents);

expect(orderChanged).toBe(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Only checks order changed, not that it's correctly sorted. Could pass even if sorting is broken.

*/
public async clickNextPage(): Promise<void> {
await this.paginationNextButton.click();
await this.waitForDagList();
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing waitForDagList() could break other tests that depend on pagination. Why was this changed?

@vatsrahul1001
Copy link
Contributor

@haseebmalik18 can you look at review comments?

@haseebmalik18
Copy link
Author

haseebmalik18 commented Jan 5, 2026

@vatsrahul1001 The E2E test for audit log pagination currently creates 4 events via 3 DAG triggers, but the UI config sets page_size: 15, so pagination won't trigger. To properly test pagination, we'd need 16+ events, which would require ~14+ DAG triggers which would be quite slow. What would you say is the recommended approach for generating event logs to generate pagination direct DB inserts, lower page_size config for tests, or another method?

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 || DAG-014: Verify DAG audit log

2 participants