Skip to content

feat(script): report unquoted non-state words as unknown state names - #4182

Merged
calmonroe merged 1 commit into
4046-make-apis-wait-check-case-insensitivefrom
4046-state-name-error-message
Oct 29, 2026
Merged

calmonroe merged 1 commit into
4046-make-apis-wait-check-case-insensitivefrom
4046-state-name-error-message

Conversation

@jmthomas

Copy link
Copy Markdown
Member

What changed

  • An unquoted word in a check, wait or wait_check comparison that is not a state name now raises NameError: Unknown state name OFFLINE. Did you mean 'OFFLINE' as a string? instead of Uninitialized constant OFFLINE... (Ruby extract.rb, Python extract.py)
  • Error type is still NameError so existing rescues keep working
  • check_expression, wait_expression and wait_check_expression keep the Uninitialized constant message since they really evaluate code
  • Added a String Comparisons note to the three expression APIs in scripting-api.md: the expression is evaluated as code, so strings must be quoted, match exactly, and state names are not case insensitive or unquotable there

Why it changed

Follow-up to #4159. Unquoted words are now valid when they are state names, so "Uninitialized constant" no longer describes the error. The expression APIs don't get the new state name behavior, which users may otherwise expect.

Testing strategy

Updated Ruby specs (extract_spec.rb, api_shared_spec.rb, 155 examples pass) and Python tests (test_extract.py, both test_api_shared.py, 202 pass). Ruff and Prettier clean.

🤖 Generated with Claude Code

An unquoted word in a check, wait or wait_check comparison is now
resolved as a state name, so the error says it is not a known state
rather than mimicking Ruby's uninitialized constant. Document that the
expression APIs evaluate code, so strings must be quoted and match exactly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 03:39
@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e7f44a9f-18d1-43b3-8953-5cfcbc9e9eaf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.37%. Comparing base (2c1225c) to head (39304c2).

Additional details and impacted files
@@                              Coverage Diff                               @@
##           4046-make-apis-wait-check-case-insensitive    #4182      +/-   ##
==============================================================================
- Coverage                                       80.62%   80.37%   -0.25%     
==============================================================================
  Files                                             913      911       -2     
  Lines                                           71322    70847     -475     
  Branches                                         2779     2741      -38     
==============================================================================
- Hits                                            57501    56946     -555     
- Misses                                          13136    13233      +97     
+ Partials                                          685      668      -17     
Flag Coverage Δ
frontend 68.59% <ø> (+0.43%) ⬆️
python 78.86% <ø> (-0.38%) ⬇️
ruby-api 83.53% <ø> (-1.85%) ⬇️
ruby-backend 86.49% <100.00%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@calmonroe
calmonroe merged commit 4fe91ed into 4046-make-apis-wait-check-case-insensitive Oct 29, 2026
39 of 49 checks passed
@calmonroe
calmonroe deleted the 4046-state-name-error-message branch October 29, 2026 19:50
calmonroe added a commit that referenced this pull request Oct 29, 2026
…4159)

* feat(script): match state names case insensitively in check and wait

- Match string operands to state names ignoring case in check, wait
  and wait_check, so 'Connected' matches CONNECTED
- Accept an unquoted state name, e.g. == Connected, instead of raising
  NameError
- Only call get_item when the operand contains a string and the type is
  not RAW, so numeric comparisons add no API call
- Add operand_contains_string and upcase_state_names to extract.rb and
  extract.py, shared by both Python api_shared copies
- Update scripting-api.md, which said state values must be quoted

Closes #4046

Co-Authored-By: Claude noreply@anthropic.com

* refactor(script): move state name comparison into extract

- Move _extract_comparison and _states into a single
  extract_comparison in extract.rb and extract.py, removing the copy
  duplicated across both Python api_shared files
- Add extract_comparison unit tests covering mixed lists, unquoted
  states, items without states and when the item is looked up
- Prefix the unused stdout block parameter in api_shared_spec.rb

Refs #4046

Co-Authored-By: Claude <noreply@anthropic.com>

* test(script): remove state tests duplicated across api_shared suites

- Keep integration coverage of case insensitive states in the api
  suite, which uses the real get_item
- Reduce the script suite to checking when API_SERVER.get_item is
  called, since extract_comparison is now unit tested in test_extract

Refs #4046

Co-Authored-By: Claude <noreply@anthropic.com>

* feat(script): accept unquoted state names inside in lists

- Resolve bare words through an optional resolver in extract_operand
  so in [On, Off] matches states, including nested lists
- Parse Python collections element by element when literal_eval
  rejects them, and look up the item only once per comparison
- Clarify that only non-state strings must be quoted in the check,
  wait and wait_check docs

Refs #4046

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* feat(script): report unquoted non-state words as unknown state names (#4182)

An unquoted word in a check, wait or wait_check comparison is now
resolved as a state name, so the error says it is not a known state
rather than mimicking Ruby's uninitialized constant. Document that the
expression APIs evaluate code, so strings must be quoted and match exactly.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jason Thomas <jason@openc3.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants