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

Test niscope fetching more thoroughly and consistently, parameterize channels tested #1956

Merged
merged 5 commits into from
May 2, 2023

Conversation

ni-jfitzger
Copy link
Collaborator

@ni-jfitzger ni-jfitzger commented Apr 29, 2023

  • This contribution adheres to CONTRIBUTING.md.
  • [ ] I've updated CHANGELOG.md if applicable.
  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

Our niscope fetch tests are inconsistent in what they test. Some, like test_read(), check very little, while others are more through. Most of them either perform the same checks or should be performing the same checks.

Additionally, we only test fetching with a single channel list.
In order to test for #1770, both issues are being addressed.

Changes:

  • Move the checking of fetched data into a separate function
    • Checking MeasurementStats has a slight difference, so check the data type to determine what checks to perform
  • Parameterize test_channels and test_channels_expanded for all of the basic fetch tests

List issues fixed by this Pull Request below, if any.

None

What testing has been done?

Ran the tests locally (with the original test_channels_2, test_channels_2_expanded values) with NI-SCOPE 23.1 driver runtime.
I confirmed that test_channels_2 causes the fetch tests to fail, catching the bug.

@ni-jfitzger
Copy link
Collaborator Author

flake8 failure occurred. Merging recent changes from main (assertion cleanup) will probably fix it.

@ni-jfitzger ni-jfitzger merged commit 195ac33 into ni:master May 2, 2023
@ni-jfitzger ni-jfitzger deleted the fix-fetch-tests branch May 2, 2023 13:24
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.

2 participants