Skip to content

Commit

Permalink
Remove/update tests which mock MRC builder objects (#955)
Browse files Browse the repository at this point in the history
* Remove tests weren't testing anything not covered in an end-to-end test, and had the possibility of triggering issue #934
* If the test was checking legitimate functionality the test was updated such that MRC internals weren't left half mocked.
* Ignore `import-error` and `no-member` errors, both of these are erroneous errors on pylints part.
* Fix what appears to be a variable name change in MRC Builder of `in_port_streams` to `to in_ports_streams`

Authors:
  - David Gardner (https://github.com/dagardner-nv)

Approvers:
  - Michael Demoret (https://github.com/mdemoret-nv)

URL: #955
  • Loading branch information
dagardner-nv authored May 22, 2023
1 parent f14eeaa commit adffc89
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 246 deletions.
7 changes: 5 additions & 2 deletions morpheus/stages/general/multi_port_modules_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""MultiPortModulesStage class."""

import logging
import typing
Expand Down Expand Up @@ -72,9 +73,11 @@ def __init__(self,

@property
def name(self) -> str:
"""Returns the name of the stage."""
return self._module_conf.get("module_name", "multi_port_module")

def supports_cpp_node(self):
"""Indicates whether the stage supports C++ node."""
return False

def input_types(self) -> typing.Tuple:
Expand Down Expand Up @@ -109,7 +112,7 @@ def _validate_ports(self, module) -> None:
raise ValueError(f"Provided output ports do not match module output ports. Module: {output_ids}, "
f"Provided: {self._out_ports}.")

def _build(self, builder: mrc.Builder, in_port_streams: typing.List[StreamPair]) -> typing.List[StreamPair]:
def _build(self, builder: mrc.Builder, in_ports_streams: typing.List[StreamPair]) -> typing.List[StreamPair]:

# Load module from the registry.
module = load_module(self._module_conf, builder=builder)
Expand All @@ -118,7 +121,7 @@ def _build(self, builder: mrc.Builder, in_port_streams: typing.List[StreamPair])

# Make an edges with input ports
for index in range(self._num_in_ports):
in_stream_node = in_port_streams[index][0]
in_stream_node = in_ports_streams[index][0]
in_port = self._in_ports[index]
mod_in_stream = module.input_port(in_port)
builder.make_edge(in_stream_node, mod_in_stream)
Expand Down
5 changes: 4 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -434,16 +434,19 @@ disable = [
"bad-inline-option",
"deprecated-pragma",
"file-ignored",
"import-error", # pylint gets confused by tests for our examples
"import-outside-toplevel", # Allow lazy imports inside of methods
"locally-disabled",
"missing-class-docstring",
"missing-function-docstring",
"missing-module-docstring",
"no-member", # pylint isn't aware of some of our attrs issue #956
"protected-access",
"raw-checker-failed",
"superfluous-parens",
"suppressed-message",
"too-many-arguments", # Disable all of the "too-many-*" checks, as they are too strict for our codebase
"too-few-public-methods", # Disable all of the "too-*" checks, as they are too strict for our codebase
"too-many-arguments",
"too-many-branches",
"too-many-instance-attributes",
"too-many-lines",
Expand Down
24 changes: 0 additions & 24 deletions tests/examples/digital_fingerprinting/test_write_to_s3_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,3 @@ def test_constructor(config: Config):

assert isinstance(stage, SinglePortStage)
assert stage._s3_writer is mock_s3_writer


@mock.patch("dfp.stages.write_to_s3_stage.ops")
def test_build_single(mock_mrc_ops, config: Config):
from dfp.stages.write_to_s3_stage import WriteToS3Stage

mock_s3_writer = mock.MagicMock()
stage = WriteToS3Stage(config, s3_writer=mock_s3_writer)

mock_mrc_map_fn = mock.MagicMock()
mock_mrc_ops.map.return_value = mock_mrc_map_fn

input_stream = (mock.MagicMock(), mock.MagicMock())

mock_node = mock.MagicMock()
mock_builder = mock.MagicMock()
mock_builder.make_node.return_value = mock_node

results = stage._build_single(mock_builder, input_stream)

assert results == (mock_node, input_stream[1])
mock_builder.make_node.assert_called_once_with(stage.unique_name, mock_mrc_map_fn)
mock_mrc_ops.map.assert_called_once_with(mock_s3_writer)
mock_builder.make_edge.assert_called_once_with(input_stream[0], mock_node)
10 changes: 1 addition & 9 deletions tests/test_directory_watcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
# limitations under the License.

import os
from unittest import mock

import pytest

Expand All @@ -30,7 +29,7 @@
@pytest.mark.parametrize('recursive', [True])
@pytest.mark.parametrize('queue_max_size', [128])
@pytest.mark.parametrize('batch_timeout', [5.0])
def test_build_node(watch_directory, max_files, sort_glob, recursive, queue_max_size, batch_timeout):
def test_constructor(watch_directory, max_files, sort_glob, recursive, queue_max_size, batch_timeout):
input_glob = os.path.join(TEST_DIRS.tests_data_dir, 'appshield', '*', '*.json')
watcher = DirectoryWatcher(input_glob,
watch_directory,
Expand All @@ -43,10 +42,3 @@ def test_build_node(watch_directory, max_files, sort_glob, recursive, queue_max_
assert watcher._sort_glob
assert watcher._watch_directory
assert watcher._max_files == -1

mock_files = mock.MagicMock()
mock_segment = mock.MagicMock()
mock_segment.make_source.return_value = mock_files

watcher.build_node('watch_directory', mock_segment)
mock_segment.make_source.assert_called_once()
32 changes: 8 additions & 24 deletions tests/test_filter_detections_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from unittest import mock

import cupy as cp
import pytest

Expand Down Expand Up @@ -122,7 +120,7 @@ def test_filter_column(config, filter_probs_df, do_copy, threshold, field_name):
# All values are at or below the threshold
output_message = fds.filter_copy(mock_message)

output_message.get_meta().to_cupy().tolist() == expected_df.to_numpy().tolist()
assert output_message.get_meta().to_cupy().tolist() == expected_df.to_numpy().tolist()


@pytest.mark.use_cudf
Expand Down Expand Up @@ -183,26 +181,12 @@ def test_filter_slice(config, filter_probs_df):

output_messages = fds.filter_slice(mock_message)
assert len(output_messages) == 2
(m1, m2) = output_messages
assert m1.offset == 2
assert m1.count == 1

assert m2.offset == 4
assert m2.count == 1

assert m1.get_meta().to_cupy().tolist() == filter_probs_df.loc[2:2, :].to_cupy().tolist()
assert m2.get_meta().to_cupy().tolist() == filter_probs_df.loc[4:4, :].to_cupy().tolist()

(msg1, msg2) = output_messages # pylint: disable=unbalanced-tuple-unpacking
assert msg1.offset == 2
assert msg1.count == 1

@pytest.mark.use_python
def test_build_single(config):
mock_stream = mock.MagicMock()
mock_builder = mock.MagicMock()
mock_builder.make_node.return_value = mock_stream
mock_stream_pair = (mock.MagicMock(), mock.MagicMock())

fds = FilterDetectionsStage(config)
fds._build_single(mock_builder, mock_stream_pair)
assert msg2.offset == 4
assert msg2.count == 1

mock_builder.make_node.assert_called_once()
mock_builder.make_edge.assert_called_once()
assert msg1.get_meta().to_cupy().tolist() == filter_probs_df.loc[2:2, :].to_cupy().tolist()
assert msg2.get_meta().to_cupy().tolist() == filter_probs_df.loc[4:4, :].to_cupy().tolist()
Loading

0 comments on commit adffc89

Please sign in to comment.