Skip to content

[java][bidi] Update tests to use converter functions - #18146

Merged
pujagani merged 1 commit into
SeleniumHQ:trunkfrom
pujagani:fix-inline-map
Oct 9, 2026
Merged

pujagani merged 1 commit into
SeleniumHQ:trunkfrom
pujagani:fix-inline-map

Conversation

@pujagani

@pujagani pujagani commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

🔗 Related Issues

Update tests to user converter functions for test related to BiDi Code generator

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

@selenium-ci selenium-ci added C-java Java Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Oct 9, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Inline synthetic BiDi maps throughout the schema

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Inline primitive-valued synthetic map references throughout the BiDi schema, including nested type
 references.
• Use the BiDi converter in NodeProperties tests to exercise production deserialization rules.
• Reject non-string attribute values with a dedicated regression test.
Diagram

graph TD
  A["BiDi schema"] --> B["Map normalization"] --> C["Type generation"] --> D["Generated BiDi types"]
  D --> E["BiDi converter"] --> F["NodeProperties tests"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Resolve synthetic maps on demand
  • ➕ Avoids copying the entire schema during generator construction.
  • ➖ Every type-reference consumer would need the same resolution logic, making nested references easier to miss.

Recommendation: Upfront normalization is the better fit for covering nested references consistently. Before merging, verify that command and event processing reads the normalized schema: the constructor still obtains those lists from its original schema argument.

Files changed (2) +57 / -32

Bug fix (1) +34 / -17
BiDiGenerator.javaNormalize synthetic map references across the schema +34/-17

Normalize synthetic map references across the schema

• Replaces field-only synthetic map inlining with a recursive schema traversal that handles references nested in maps and lists. The generator builds its type table from the transformed schema, while primitive-valued synthetic maps remain the only eligible maps.

java/src/org/openqa/selenium/bidi/BiDiGenerator.java

Tests (1) +23 / -15
NodePropertiesTest.javaTest attributes through the BiDi converter +23/-15

Test attributes through the BiDi converter

• Switches attribute deserialization tests from a default Json instance to ConverterFunctions.JSON. Adds a case asserting that numeric attribute values are rejected.

java/test/org/openqa/selenium/bidi/protocol/script/NodePropertiesTest.java

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@pujagani
pujagani merged commit 73b2084 into SeleniumHQ:trunk Oct 9, 2026
49 checks passed
This was referenced Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants