Skip to content

[Task]: Add tests for the key handlers #43

Description

@fedonman

What needs doing

Start testing the key handlers. There are six public handle_key methods in the project and not one test presses a key. The ones worth starting with need no terminal and no cluster: FilterTree::handle_key, which takes &mut QueryParams and returns an enum, FieldSelector::handle_key, which mutates only its own lists, WidgetSelector::handle_key, which takes &mut VisibleWidgets, and the follow-mode keys on the output panes.

Add crossterm to [dev-dependencies] as the first step. There is no [dev-dependencies] section at all today, so an integration test under tests/ cannot construct a KeyEvent even though the type is public, and the tests would otherwise have to live as #[cfg(test)] modules inside the view files. That is a likelier reason this has not happened than nobody having thought of it.

Why

Keybindings are the whole interface, and they are the cheapest thing in a TUI to test because these handlers already take their entire world as arguments. None of them needs a Frame.

The gap has cost something already. #40, where Ctrl+S mid-edit saves the previous value and reports success, is about four lines of test away from being caught, and #39's missing modifier guard is two.

JobTable::set_jobs is worth doing in the same pass. Marks and cursor surviving a refresh is the headline behaviour and has no coverage: feed it two job lists and assert the cursor follows the job id, the marks are remapped, and a focused job that has vanished falls back sanely.

One more worth adding: assert every key named in help_lines appears somewhere in README.md. That direction catches a binding that moves or is added, which is the drift that produced #27. The reverse direction would false-fail, because help_lines does not cover the column dialog or the widget selector while the README does.

The dashboard's own dispatch comes last. on_keypress is private and Dashboard::new calls check_slurm_available before anything else, so it needs the harness in #42 or the SLURM-free AppState split. The difficulty of testing it is itself an argument for that refactor rather than a separate motivation for it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    taskA chore, refactor, deprecation or performance changetesting-ciTouches the test suite or the CI workflows

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions