nirfsg: Enable complex number support for gRPC and update system tests#2172
nirfsg: Enable complex number support for gRPC and update system tests#2172
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2172 +/- ##
==========================================
+ Coverage 88.76% 89.76% +0.99%
==========================================
Files 73 73
Lines 18986 19004 +18
==========================================
+ Hits 16853 17059 +206
+ Misses 2133 1945 -188
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
build/templates/_grpc_stub_interpreter.py/numpy_read_method.py.mako
Outdated
Show resolved
Hide resolved
build/templates/_grpc_stub_interpreter.py/numpy_read_method.py.mako
Outdated
Show resolved
Hide resolved
build/templates/_grpc_stub_interpreter.py/numpy_read_method.py.mako
Outdated
Show resolved
Hide resolved
build/templates/_grpc_stub_interpreter.py/numpy_read_method.py.mako
Outdated
Show resolved
Hide resolved
|
You should add new tests to src\nifake\unit_tests\test_grpc.py |
|
I'm worried about the system test results. We seem to have hung as soon as we reached the first nirfsg gRPC test for 32-bit. You may want to investigate on a local system. |
| def session_creation_kwargs(self): | ||
| return {} | ||
|
|
||
| def test_get_all_named_waveform_names(self, rfsg_device_session): |
There was a problem hiding this comment.
Can you add a comment about why this test was moved to TestLibrary and when it can be moved back?
There was a problem hiding this comment.
Tried to move this back within the common path to check whether the failure in system test is anyway related to this. But it doesnt seems to be. Will investigate further regarding the nirfsg system test failure and update again.
…t_get_all_named_waveform_names in test_system_nirfsg to common path for testing
| # Re-enabled for 32-bit testing after gRPC device update | ||
| @pytest.mark.skipif(use_simulated_session is True, reason="Needs Updated gRPC device that supports get_all_named_waveform_names bug fix") | ||
| def test_get_all_named_waveform_names(self, rfsg_device_session): | ||
| rfsg_device_session.generation_mode = nirfsg.GenerationMode.ARB_WAVEFORM | ||
| waveform_data1 = np.full(1000, 1 + 0j, dtype=np.complex128) | ||
| waveform_data2 = np.full(800, 1 + 0j, dtype=np.complex128) | ||
| rfsg_device_session.write_arb_waveform('waveform1', waveform_data1, False) | ||
| rfsg_device_session.write_arb_waveform('waveform2', waveform_data2, False) | ||
| names = rfsg_device_session.get_all_named_waveform_names() | ||
| assert 'waveform1' in names | ||
| assert 'waveform2' in names |
There was a problem hiding this comment.
Unless I'm misinterpreting this, now you're skipping this for non-gRPC cases, too?
|
travis says you need to run codegen again. |
I've updated CHANGELOG.md if applicable.What does this Pull Request accomplish?
numpy_read_method.py.makotemplate with snippets copied over fromdefault_method.py.makoand also included gRPC generation support for numpy complex methods._grpc_stub_interpreter.py.makotemplate to importnidevice_pb2 as grpc_complex_typesonly if complex numbers are used in the module.nifake.protofile to generate the necessary changes to include in the nifake unit tests.grpc_server_config.jsonfile for nirfsg to include the address and port for server listening.List issues fixed by this Pull Request below, if any.
NA
What testing has been done?
TestGrpcclass intest_system_nirfsg.pyto include gRPC-based system test session setup.test_get_all_named_waveform_namestest will be only passing when the next release version of grpc-device server is used for nimi-python system tests. So this test case was moved from common path to TestLibrary class intest_system_nirfsg.pyfile to skip being tested against grpc. This movement will be reverted once we update the grpc-device version being used later.use_simulated_sessionoption as False, all the nirfsg test cases are passing as expected.