Pushkal-Gupta commented on code in PR #76096:
URL: https://github.com/apache/airflow/pull/76096#discussion_r4436474470
##########
airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstancesFilter.test.tsx:
##########
@@ -32,7 +32,7 @@ describe("Task Instances date filters", () => {
const pill = await screen.findByTestId("logical_date_range-pill");
expect(pill).toBeInTheDocument();
- expect(screen.getByRole("button", { name: /Remove .* filter/u
})).toBeInTheDocument();
+
expect(screen.getByTestId("logical_date_range-pill-remove")).toBeInTheDocument();
Review Comment:
You're right that the test id gives up the name check, so I've put one back.
But the fix doesn't work though, and the case isn't the reason.
I dumped what the button is actually called in that test:
```
aria-label: "filters.removeNamedFilter"
```
It's the bare key. `AppWrapper` pulls the locale files over HTTP and jsdom
doesn't serve them, so `t()` has nothing to resolve. That's also why the old
`/Remove .* filter/u` used to pass: it was matching the hardcoded English, not
a translation. Now that the name comes from the locale there's no English
wording here to match at all, in either case.
So I did what the three new tests in this PR do and registered the bundle
first:
```tsx
beforeEach(() => {
i18n.addResourceBundle(
"en",
"common",
{ filters: { removeNamedFilter: "Drop the {{label}} filter" } },
true,
true,
);
});
```
```tsx
expect(screen.getByRole("button", { name: /^Drop the .+ filter$/u
})).toBeInTheDocument();
```
Slightly better than what was there before, since "Drop the ... filter"
isn't something the component could say by accident. Revert `FilterPill.tsx`
and it fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
