-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add integration tests for /events Audit log page (Part 1) #60122
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
base: main
Are you sure you want to change the base?
Conversation
71c92e3 to
8b842f2
Compare
|
@vatsrahul1001 can you take a look. Builds are passing now. Thanks |
|
Thanks, I will review it soon! |
|
@Prajwal7842 Thanks for the PR! A few things to address:
Let me know if you have questions! |
| await eventsPage.navigate(); | ||
| await eventsPage.waitForEventsTable(); | ||
|
|
||
| await eventsPage.waitForTimeout(5000); |
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.
This adds 10 seconds of unnecessary wait time
await expect(async () => {
const sortIndicator = await eventsPage.getColumnSortIndicator(0);
expect(sortIndicator).not.toBe("none");
}).toPass({ timeout: 10_000 });
| test.setTimeout(3 * 60 * 1000); // 3 minutes timeout | ||
|
|
||
| // First, trigger a DAG to generate audit log entries | ||
| await dagsPage.triggerDag(testDagId); |
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.
Move trigger dag to beforeAll
test.describe("Events with Generated Data", () => {
const testDagId = testConfig.testDag.id;
test.beforeAll(async ({ browser }) => {
test.setTimeout(3 * 60 * 1000);
const context = await browser.newContext({ storageState: AUTH_FILE });
const page = await context.newPage();
const dagsPage = new DagsPage(page);
await dagsPage.triggerDag(testDagId);
await context.close();
});
test.beforeEach(({ page }) => {
eventsPage = new EventsPage(page);
});
| this.eventsTable = page.getByRole("table"); | ||
| this.filterBar = page.locator('div:has(button:has-text("Filter"))').first(); | ||
| // Use table header selectors - will check for presence of key columns | ||
| this.timestampColumn = page.locator("thead th").first(); |
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.
If column order changes, all tests break.
Maybe we can use something like below
this.timestampColumn = page.getByRole("columnheader", { name: /when/i });
this.eventTypeColumn = page.getByRole("columnheader", { name: /event/i });
this.userColumn = page.getByRole("columnheader", { name: /user/i });
| await expect(eventsPage.eventsTable).toBeVisible({ timeout: 10_000 }); | ||
| }); | ||
|
|
||
| test("should verify column sorting works", async () => { |
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.
This doesn't verify the data is actually sorted.
Capture data before/after and verify order
| eventsPage = new EventsPage(page); | ||
| }); | ||
|
|
||
| test("should verify search input is visible", async () => { |
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.
Test only checks that the filter UI exists:
- Opens filter menu
- Verifies menu items visible
- Closes menu
This is NOT a filter test - it's just checking the filter UI is present.
Thanks @vatsrahul1001 for the review. I have addressed the comments. |
28790a3 to
b76c454
Compare
This PR adds playwright integration tests for /events Audit log page for Airflow UI.
Covers following things from listed down in the issues:
Audit Logs Display
Search/Filter Logs
closes: #59931
releated: #59028
The integration tests were locally run 5+ times consecutively and all runs passed successfully to avoid any flakiness

Sample run: