Skip to content

unit test: Introduce python test for nislsc modules - #63

Merged
junliang-tan merged 8 commits into
mainfrom
users/zchan/nislsc_python_test
Jul 24, 2026
Merged

unit test: Introduce python test for nislsc modules#63
junliang-tan merged 8 commits into
mainfrom
users/zchan/nislsc_python_test

Conversation

@zoechanzy

@zoechanzy zoechanzy commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

What does this Pull Request accomplish?

  • Created pytest unit tests for:
    • command module
    • error module
    • library module
    • property module
    • session module
    • utils module
  • Updated pyproject.yaml to include pytest dependencies

Why should this Pull Request be merged?

Add unit test to ensure we have test coverage for slsc modules.

What testing has been done?

Ran unit test locally using poetry run pytest tests/unit/ and all the tests passed.
unit test result

zoechanzy added 5 commits July 7, 2026 13:53
* Add test_command.py for command module
* Add test_error.py for error module
* Add test_library.py for library module
* Add test_property.py for property module
* Add test_session.py for session module
* Add test_utils.py for utils module

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Refactor test name to follow pattern
  test___<precondition>___<action>___<outcome>

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Add tests to increase test coverage for session module

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Refactor tests using pytest.mark.parametrize to reduce duplication

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Add
  test___open_device_command_open___close_command___interpreter_close_command_called
  for command module
* Add
  test___interpreter_available___get_library_version___delegates_to_interpreter
  for utils module

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
@zoechanzy
zoechanzy force-pushed the users/zchan/nislsc_python_test branch from bcfdf5d to 480b8d8 Compare July 7, 2026 07:24
@zoechanzy zoechanzy changed the title Users/zchan/nislsc python test unit test: Introduce unit test for nislsc modules Jul 7, 2026
@zoechanzy zoechanzy changed the title unit test: Introduce unit test for nislsc modules unit test: Introduce python test for nislsc modules Jul 7, 2026
@zoechanzy
zoechanzy force-pushed the users/zchan/nislsc_python_test branch from 480b8d8 to 5178e2f Compare July 8, 2026 01:53
* Add pytest dependencies
* Update python constraints to docs dependencies
* Regenerate lock file
* Reformat unit test files for styleguide compliance
---------

Sign-off-by: zoechanzy <zoe.chan@emerson.com>
@zoechanzy
zoechanzy force-pushed the users/zchan/nislsc_python_test branch from 5178e2f to dbd9e8e Compare July 8, 2026 04:55
@zoechanzy
zoechanzy requested a review from Copilot July 8, 2026 05:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a pytest-based unit test suite to validate the generated nislsc Python API surface (largely via interpreter delegation/mocking), and updates Poetry configuration to support running those tests in development.

Changes:

  • Added unit tests for Library, Session, Command, Property, utils, and error/warning types using pytest + pytest-mock.
  • Introduced shared test fixtures/helpers (tests/unit/conftest.py, _session_utils.py) to standardize interpreter/library/session setup.
  • Added a Poetry test dependency group (pytest/pytest-mock/pytest-cov) and refreshed poetry.lock.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/unit/test_utils.py Unit tests for utils helpers delegating to the interpreter.
tests/unit/test_session.py Large session API delegation and error/warning behavior tests.
tests/unit/test_property.py Property open/close and property metadata delegation tests.
tests/unit/test_library.py Library lifecycle, language, and error description tests.
tests/unit/test_error.py Error/warning type behavior and formatting tests.
tests/unit/test_command.py Command open/close and command metadata delegation tests.
tests/unit/conftest.py Shared pytest fixtures (mock interpreter, library, session).
tests/unit/_session_utils.py Helper functions for configuring interpreter return values.
tests/unit/init.py Marks unit test package.
tests/init.py Marks tests package.
pyproject.toml Adds test dependency group; refines docs deps python markers.
poetry.lock Lockfile update reflecting new dependency group(s) and resolver output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit/test_session.py Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@zoechanzy
zoechanzy marked this pull request as ready for review July 8, 2026 05:51
Comment thread tests/unit/test_utils.py
* Remove flatten and unflatten tests variants in test_utils.py
* Remove redundant test in test_command.py
* Parametrize tests in test_property.py

---------

Signed-off-by: zoechanzy <zoe.chan@emerson.com>
@ni-github-admins

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@zoechanzy
zoechanzy requested a review from tjying95 July 17, 2026 10:10
@tjying95

Copy link
Copy Markdown
Collaborator

I would prefer to have the unit test action be enabled here, but I see #64 so it's probably fine.

@junliang-tan

Copy link
Copy Markdown
Collaborator

Personally, I would suggest splitting the refactoring changes in commit dbd9e8ef1b5c3c5d44c518923c6334760f4c92d4 into a separate commit.
Since the changes will be squashed during the merge, it does not make much difference in this case.
However, keeping refactoring and functional changes in separate commits is generally recommended.

@junliang-tan
junliang-tan self-requested a review July 22, 2026 09:01
@bkeryan

bkeryan commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

I didn't review all of the new tests in detail but I skimmed them and they look good.

Have you been using https://pypi.org/project/pytest-cov/ to verify which methods have test coverage?

@zoechanzy

zoechanzy commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator Author

I didn't review all of the new tests in detail but I skimmed them and they look good.

Have you been using https://pypi.org/project/pytest-cov/ to verify which methods have test coverage?

Thanks for the pytest-cov suggestion. It gave me a proper way on checking the test coverage.
test-coverage

I think 0% test coverage for generated\nislsc_library_interpreter.py are expected in unit tests because unit tests mock BaseInterpreter instead of importing the ctypes wrapper. nislsc_library_interpreter.py can be tested in system or functional tests with real or simulated devices.

67% for _base_interpreter.py is also expected because abstract pass method bodies are unreachable in unit tests.

@junliang-tan
junliang-tan merged commit 839404e into main Jul 24, 2026
3 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.

6 participants