Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: make tst_pim_2 loadable, types.SimpleNamespace does not take positional args #349

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

tangkong
Copy link
Contributor

@tangkong tangkong commented Oct 15, 2024

Description

In conftest.py, we defined a mock database with some SimpleNamespace devices.

Motivation and Context

Jane ran into this, uncovering a poorly written test (very likely my fault)

Many of the tests in happi are also quite brittle. I'd love to blame someone else for this but I did go through rewriting tests for click and did not fix this problem.

How Has This Been Tested?

pytest

Where Has This Been Documented?

This PR

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

@ZLLentz
Copy link
Member

ZLLentz commented Oct 15, 2024

One way to make this specific test less brittle is to refactor so that the source dictionaries for the db fixtures are defined in an importable function that other tests can call and check the original source json with.

@tangkong tangkong requested a review from janeliu-slac October 15, 2024 17:58
@tangkong tangkong merged commit 886aa25 into pcdshub:master Oct 15, 2024
11 checks passed
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