Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions airgun/entities/host_new.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ def create(self, values):
host_view.flash.assert_no_error()
host_view.flash.dismiss()

def search_autocomplete(self, value):
"""Search the All Hosts page and return autocomplete suggestions or error messages."""
view = self.navigate_to(self, 'NewUIAll')
view.searchbox.clear()
view.searchbox.fill(value)
results = (
wait_for(
view.autocomplete_menu.read,
timeout=5,
delay=0.5,
handle_exception=True,
silent_failure=True,
)[0]
or []
Comment on lines +70 to +78

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.

issue (bug_risk): Clarify handling of failures and empty results from wait_for

Because handle_exception=True and silent_failure=True cause wait_for to return (None, ...) on any exception or timeout, this method will always return [] in those cases. That makes a genuine “no suggestions” case indistinguishable from a failure to show/read the menu. If you need to detect or debug UI issues, consider either not silencing failures here or returning something richer (e.g., distinguish None from [] or add a status flag) so callers can assert on unexpected conditions.

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.

Do we need 'or' section. Because as sourcery-ai suggest handle_exception=True and silent_failure=True will return "None" on any exception or timeout. Have you verified it?

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.

Good question. The short answer is: the or [] is not strictly required for correctness, but it is doing something slightly different from what my earlier comment emphasized.

With this line:

results = (
    wait_for(
        view.autocomplete_menu.read,
        timeout=5,
        delay=0.5,
        handle_exception=True,
        silent_failure=True,
    )[0]
    or []
)

we have two distinct behaviors:

  1. Normal case (no timeout/exception):
    wait_for(...)[0] will be whatever view.autocomplete_menu.read() returns (presumably a list of suggestions, or possibly [] if there are none). In that case the or [] is effectively a no‑op.

  2. Failure/timeout case:
    With handle_exception=True and silent_failure=True, wait_for will return (None, ...) when it times out or hits an exception. Then wait_for(...)[0] is None, and the or [] converts that to [].

So:

  • Yes, you are right that wait_for returns None in those cases.
  • The or [] is there to normalize None to an empty list, so callers always see list rather than None.

Whether we need that depends on what the tests and callers are expecting:

  • If the test logic asserts only on the suggestions (e.g. assert [] when nothing is shown), the current or [] is fine but masks the difference between no suggestions and autocomplete failed to render.
  • If we want to be able to detect timeouts or failures separately, we should remove silent_failure=True or return a richer structure, for example:
    suggestions, meta = wait_for(...)
    return {
        "suggestions": suggestions or [],
        "timed_out": meta and meta.get("timed_out", False),
    }

Given your question, if the intent of this helper is just: "give me suggestions or an empty list", then the or [] is still useful to keep the return type stable. If instead you want the caller to know about failures, then you can safely drop silent_failure=True and/or the or [] and let None bubble up.

If you’d like, I can suggest a small refactor that keeps type‑safety (always list) but also makes it easier to debug failures.

)
return results

def get_details(self, entity_name, widget_names=None):
"""Read host values from Host Details page, optionally only the widgets in widget_names will be read."""
view = self.navigate_to(self, 'NewDetails', entity_name=entity_name)
Expand Down
3 changes: 2 additions & 1 deletion airgun/views/all_hosts.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
WizardStepView,
)
from airgun.views.host_new import ManageColumnsView, PF5CheckboxTreeView
from airgun.widgets import ItemsList, SearchInput
from airgun.widgets import ItemsList, PF5NavSearchMenu, SearchInput


class MenuToggleDropdownInTable(PF5Dropdown):
Expand Down Expand Up @@ -122,6 +122,7 @@ class AllHostsTableView(BaseLoggedInView, SearchableViewMixinPF4):
2: MenuToggleDropdownInTable(),
},
)
autocomplete_menu = PF5NavSearchMenu(component_id='search-autocomplete-menu')
alert_message = Text('.//div[contains(@class, "pf-v5-c-alert")]')

# Host status icon and popover widgets
Expand Down
Loading