infra: Webui sizzle remove by adamkankovsky · Pull Request #1242 · rhinstaller/anaconda-webui

adamkankovsky · 2026-04-07T11:40:17Z

No description provided.

sourcery-ai

Hey - I've left some high level feedback:

The new check_disk_row/check_disk_row_not_present implementations rely on substring matches in textContent across the entire row, which is looser than the previous cell-based selectors and may produce false positives when multiple disks share similar values; consider tightening the matching (e.g., per-cell checks or stricter delimiters) if that is important for these tests.
The inline JavaScript snippets for wait_js_cond in check_disk_row and check_disk_row_not_present are very similar; consider factoring the common logic into a small helper to keep the test helpers easier to maintain.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `check_disk_row`/`check_disk_row_not_present` implementations rely on substring matches in `textContent` across the entire row, which is looser than the previous cell-based selectors and may produce false positives when multiple disks share similar values; consider tightening the matching (e.g., per-cell checks or stricter delimiters) if that is important for these tests.
- The inline JavaScript snippets for `wait_js_cond` in `check_disk_row` and `check_disk_row_not_present` are very similar; consider factoring the common logic into a small helper to keep the test helpers easier to maintain.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

gemini-code-assist

Code Review

This pull request removes the Sizzle dependency, updates the Cockpit repository commit, and refactors test helpers to eliminate Sizzle-based selectors. In test/helpers/network.py, methods now use NetworkCase via a shim, while test/helpers/review.py replaces CSS selectors with custom JavaScript conditions. Feedback indicates a potential bug in the construction of search fragments where None values could be incorrectly processed as strings. Additionally, the new JavaScript-based row validation is less precise than the original column-specific selectors, which may lead to false positives in tests.

gemini-code-assist · 2026-04-07T11:41:37Z

test/helpers/review.py

+        fragments = []
+        for part in (parent, str(size) if size != "" else "", action, encrypt_text, mount_point):
+            if part is not None and str(part) != "":
+                fragments.append(str(part))


The logic for building the fragments list is redundant and contains a potential bug. The ternary expression str(size) if size != "" else "" will convert None to the string "None", which then passes the subsequent check and is added to the search fragments. Since the loop already handles string conversion and filters out empty values, this can be simplified into a list comprehension.

fragments = [str(p) for p in (parent, size, action, encrypt_text, mount_point) if p is not None and str(p) != ""]

gemini-code-assist · 2026-04-07T11:41:37Z

test/helpers/review.py

+            cond = """(() => {
+              const table = document.querySelector(%s);
+              if (!table) return false;
+              const frags = %s;
+              const rows = table.querySelectorAll("tbody tr");
+              for (let i = 0; i < rows.length; i++) {
+                const t = rows[i].textContent;
+                if (frags.every(f => t.includes(f))) return true;
+              }
+              return false;
+            })()""" % (sel_json, frag_json)
+        else:
+            idx = int(rowIndex) - 1
+            cond = """(() => {
+              const table = document.querySelector(%s);
+              if (!table) return false;
+              const frags = %s;
+              const rows = table.querySelectorAll("tbody tr");
+              const tr = rows[%d];
+              if (!tr) return false;
+              const t = tr.textContent;
+              return frags.every(f => t.includes(f));
+            })()""" % (sel_json, frag_json, idx)
+        self.browser.wait_js_cond(cond)


The new implementation using textContent.includes() is significantly less precise than the original Sizzle-based selector. The original code used the adjacent sibling combinator (+) to ensure that the expected values appeared in specific columns and in a specific order. The current JavaScript logic will match if the fragments appear anywhere in the row in any order, which increases the risk of false positives in tests. Consider implementing a check that validates the order of fragments within the row text or checks individual cells.

KKoukiou

I think the last commit might not be needed?

I see cockpit supports it's own contains:

test/common/test-functions.js: // Best effort support :contains()

adamkankovsky · 2026-04-08T13:08:26Z

I don't think the declining test scores have anything to do with the change.

KKoukiou

The failing tests look relevant. Please rebase and see actual results.

In any case, the test suite was stabilized today, if you see errors after rebasing that must be a problem here.

Remove the unused Sizzle devDependency and refresh the node_modules cache. Cockpit test-functions only supports a single :contains() per selector when Sizzle is absent. Chained td:contains in check_disk_row therefore fails with "Unsupported multiple ':contains'". - review: assert disk table rows via wait_js_cond over #storage-review-table-<disk> tbody tr text; fix rowIndex as nth data row (tbody tr), not tbody:nth-child. - network: call NetworkCase.configure_iface_setting / wait_for_iface_setting through a SimpleNamespace(browser=...) so behaviour matches netlib without duplicating [data-label] selectors. Made-with: Cursor

Rewrite StorageReclaimDialog to use wait_js_cond and button matching by aria-label instead of chained tr/td :contains selectors (Cockpit testlib without Sizzle). Made-with: Cursor

Rewrite StorageReclaimDialog to use wait_js_cond and button matching by aria-label instead of chained tr/td :contains selectors (Cockpit testlib without Sizzle).

sourcery-ai bot reviewed Apr 7, 2026

View reviewed changes

gemini-code-assist bot reviewed Apr 7, 2026

View reviewed changes

adamkankovsky force-pushed the webui-sizzle-remove branch from 361f8d9 to c3bd0bb Compare April 7, 2026 11:47

KKoukiou reviewed Apr 7, 2026

View reviewed changes

adamkankovsky force-pushed the webui-sizzle-remove branch from c3bd0bb to 9e3a5fa Compare April 7, 2026 12:02

adamkankovsky added the node_modules label Apr 7, 2026

adamkankovsky temporarily deployed to npm-update April 7, 2026 12:08 — with GitHub Actions Inactive

github-actions bot removed the node_modules label Apr 7, 2026

adamkankovsky force-pushed the webui-sizzle-remove branch from 9e3a5fa to 2373936 Compare April 7, 2026 12:08

adamkankovsky added the node_modules label Apr 7, 2026

adamkankovsky temporarily deployed to npm-update April 7, 2026 12:10 — with GitHub Actions Inactive

github-actions bot removed the node_modules label Apr 7, 2026

KKoukiou added the node_modules label Apr 7, 2026

KKoukiou temporarily deployed to npm-update April 7, 2026 12:22 — with GitHub Actions Inactive

github-actions bot removed the node_modules label Apr 7, 2026

adamkankovsky force-pushed the webui-sizzle-remove branch 2 times, most recently from 76af1a1 to 433dd68 Compare April 7, 2026 12:41

KKoukiou added the .github-changes Set by a reviewer just before landing to acknowledge that a PR changes github workflows label Apr 7, 2026

adamkankovsky force-pushed the webui-sizzle-remove branch from 433dd68 to 4bb8dda Compare April 7, 2026 12:51

github-actions bot removed the .github-changes Set by a reviewer just before landing to acknowledge that a PR changes github workflows label Apr 7, 2026

adamkankovsky force-pushed the webui-sizzle-remove branch 2 times, most recently from 6a88bfc to cc834ac Compare April 8, 2026 11:52

adamkankovsky requested a review from KKoukiou April 8, 2026 13:08

KKoukiou reviewed Apr 10, 2026

View reviewed changes

adamkankovsky force-pushed the webui-sizzle-remove branch 2 times, most recently from f926447 to fbbcde8 Compare April 14, 2026 07:48

adamkankovsky added the node_modules label Apr 14, 2026

adamkankovsky temporarily deployed to npm-update April 14, 2026 07:50 — with GitHub Actions Inactive

github-actions bot removed the node_modules label Apr 14, 2026

adamkankovsky force-pushed the webui-sizzle-remove branch from fbbcde8 to df55545 Compare April 14, 2026 07:55

adamkankovsky added the node_modules label Apr 14, 2026

adamkankovsky deployed to npm-update April 14, 2026 07:58 — with GitHub Actions Active

github-actions bot removed the node_modules label Apr 14, 2026

adamkankovsky added 2 commits April 14, 2026 10:41

test: reclaim modal helpers without Sizzle :contains

a46ebed

Rewrite StorageReclaimDialog to use wait_js_cond and button matching by aria-label instead of chained tr/td :contains selectors (Cockpit testlib without Sizzle). Made-with: Cursor

adamkankovsky force-pushed the webui-sizzle-remove branch from 544fc0e to a46ebed Compare April 14, 2026 08:42

test: reclaim modal helpers without Sizzle :contains

e7e68ca

Rewrite StorageReclaimDialog to use wait_js_cond and button matching by aria-label instead of chained tr/td :contains selectors (Cockpit testlib without Sizzle).

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

infra: Webui sizzle remove#1242

infra: Webui sizzle remove#1242
adamkankovsky wants to merge 3 commits intorhinstaller:mainfrom
adamkankovsky:webui-sizzle-remove

adamkankovsky commented Apr 7, 2026

Uh oh!

sourcery-ai bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Apr 7, 2026

Uh oh!

gemini-code-assist bot Apr 7, 2026

Uh oh!

KKoukiou left a comment

Uh oh!

adamkankovsky commented Apr 8, 2026

Uh oh!

KKoukiou left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

adamkankovsky commented Apr 7, 2026

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

KKoukiou left a comment

Choose a reason for hiding this comment

Uh oh!

adamkankovsky commented Apr 8, 2026

Uh oh!

KKoukiou left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KKoukiou left a comment •

edited

Loading