ECS-6433 heuristic loading #231

[janeliu-slac]

Description

• Updated happi's load function to support searching and loading in one user-friendly expression. The load function gathers the results of each search term individually and loads the union of the results.
• Cleaned up code by removing stray print statements.
• Added unit tests for load function.
• In load function of cli.py removed term=[] and replaced with [item] for readability.

Motivation and Context

It allows users to search and load device objects using one CLI expression.

#231

How Has This Been Tested?

Manual testing and unit tests.

Where Has This Been Documented?

Documentation is in the code and release notes.

Pre-merge checklist

• Code works interactively
• Code contains descriptive docstrings, including context and API
• New/changed functions and methods are covered in the test suite where possible
• Test suite passes locally
• Test suite passes on GitHub Actions
• Ran docs/pre-release-notes.sh and created a pre-release documentation page
• Pre-release docs include context, functional descriptions, and contributors as appropriate

[tangkong]

As an aside, I think it's worthwhile developing the tests as you write the code. I'm often guilty of writing code then making tests after, but practicing proper test-driven development can often help speed up your development.

In this case, if you set up tests that ran the happi load CLI with various argument combinations, you'd have uncovered some of the bugs I mentioned above

[janeliu-slac]

I updated the test_load_glob_args function so for res in results will print out the name of each of the mock devices, but got stuck trying to get those names into runner.invoke(happi_cli, ['--path', happi_cfg, 'load', name1, name2]). Is there a way to add these names without hardcoding it into runner.invoke? Currently, client.search_regex(name='tst_base_.*') generates 2 results, but there may be more if someone adds more mock devices in the future.

def test_load_glob_args(
    caplog: pytest.LogCaptureFixture,
    client: happi.client.Client,
    happi_cfg: str,
    runner: CliRunner
):
    # get search results
    results = client.search_regex(name='tst_base_.*')

    for res in results:
        name = res.item['name']  # the results are 'tst_base_pim' and 'tst_base_pim2'
        print(name)

    # try to load the item
    with mock.patch.object(IPython, 'start_ipython') as m:
        _ = runner.invoke(
            happi_cli, ['--path', happi_cfg, 'load', 'tst_base_pim', 'tst_base_pim2']
        )
        m.assert_called_once_with(argv=['--quick'], user_ns=devices)        
    with caplog.at_level(logging.INFO):
        assert "Creating shell with devices" in caplog.text

[ZLLentz]

Hi Jane, if you want to discuss your code you should push it so we can make in-line comments.
The test you show here calls runner.invoke without using the glob so I'm confused

[janeliu-slac]

It seems like client.load_device() isn't able to accept globs so I used client.search_regex(name='tst_base_.*'), which returns multiple SearchResult objects. I was able to get name from SearchResult objects but wasn't sure how to pass them into runner.invoke(). I rewrote the code and just commited it (it's definitely not passing pytest).

[ZLLentz]

I think I'm happy with this, I'll leave my approval prior to my vacay

The merge-base changed after approval.

[tangkong]

I swear these are my last nitpicks, then I think we're good to go

[janeliu-slac]

Lol it's fine. I'm learning a lot and taking notes for future projects.

[tangkong]

LGTM 👍

The merge-base changed after approval.

The merge-base changed after approval.