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.

Sign up for GitHub

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

Jump to bottom

Remove/update tests which mock MRC builder objects #955

Merged
merged 23 commits into from
May 22, 2023
Merged

Remove/update tests which mock MRC builder objects #955

merged 23 commits into from
May 22, 2023

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented May 19, 2023 •
edited
Loading

Description

  • Remove tests weren't testing anything not covered in an end-to-end test, and had the possibility of triggering issue [BUG]: tests/test_dfp.py intermittently Segmentation faults #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

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

dagardner-nv and others added 12 commits May 17, 2023 08:03
@dagardner-nv
Temp hard-code debug logging - DO NOT MERGE
ffcd5e6
@dagardner-nv
Temp testing - DO NOT MERGE
ed6a1b4
@dagardner-nv
Add pybind to link targets, this is needed when MRC is pulled in via cpm
c85b560
@dagardner-nv
Remove problematic test which mocked the MRC builder object, but not the
030e807 
operators. This ended up with an unexpected situation of MRC operators and
thus instances of PyFuncWrapper which were left orphaned and not destructed
properly.

This test wasn't testing much that wouldn't already be covered by our
integration tests, and was causing segfaults elsewhere covered in
issue 934
@dagardner-nv
Revert "Temp testing - DO NOT MERGE"
b4718b2 
This reverts commit ed6a1b4.
@dagardner-nv
Revert "Temp hard-code debug logging - DO NOT MERGE"
e45f495 
This reverts commit ffcd5e6.
@dagardner-nv
Merge branch 'branch-23.07' into david-segfaults-934
25b8ce9
@dagardner-nv
Remove unused import
2aeb918
@dagardner-nv
Merge branch 'david-segfaults-934' of github.com:dagardner-nv/Morpheu…
7c4e2f5 
…s into david-segfaults-934
@dagardner-nv
Mock MonitorController methods rather than MRC
a7ecf75
@dagardner-nv
Remove tests of little value which mock MRC's builder to avoid issues…
6f57229 
… like nv-morpheus#934
@dagardner-nv
Merge branch 'branch-23.07' of github.com:nv-morpheus/Morpheus into d…
cb1c075 
…avid-remove-mocked-builders
@dagardner-nv dagardner-nv requested a review from a team as a code owner May 19, 2023 22:21
@dagardner-nv dagardner-nv added bug Something isn't working non-breaking Non-breaking change 2 - In Progress labels May 19, 2023
@dagardner-nv dagardner-nv requested a review from a team as a code owner May 19, 2023 23:22
mdemoret-nv
mdemoret-nv approved these changes May 22, 2023
View reviewed changes
@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit adffc89 into nv-morpheus:branch-23.07 May 22, 2023
@dagardner-nv dagardner-nv deleted the david-remove-mocked-builders branch May 22, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdemoret-nv mdemoret-nv mdemoret-nv approved these changes

Assignees
No one assigned
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
Morpheus Boards
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dagardner-nv @mdemoret-nv