-
Notifications
You must be signed in to change notification settings - Fork 162
Refactored Unit Test Startup and Teardown #916
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #916 +/- ##
==========================================
- Coverage 80.15% 80.11% -0.05%
==========================================
Files 128 128
Lines 37242 37244 +2
==========================================
- Hits 29851 29837 -14
- Misses 7391 7407 +16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good improvement for test stability.
Made some comments on unclear parts.
_unittest/conftest.py
Outdated
def my_teardown(self, close_desktop=False): | ||
if close_desktop and not is_ironpython: | ||
d = Desktop(desktop_version, non_graphical, False) | ||
d.release_desktop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add also del self.aedtapps
and del self.edbapps
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. Thanks
_unittest/conftest.py
Outdated
|
||
def add_app(self, project_name=None, design_name=None, solution_type=None, application=None): | ||
if "oDesktop" not in dir(sys.modules["__main__"]): | ||
if self.aedtapps: | ||
self.aedtapps[0].release_desktop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a safety mechanism?
I don't get when it will execute this release_desktop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right. it should be in the teardown method.
_unittest/conftest.py
Outdated
# desktop = Desktop(desktop_version, non_graphical, new_thread) | ||
# desktop.disable_autosave() | ||
# yield desktop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# desktop = Desktop(desktop_version, non_graphical, new_thread) | |
# desktop.disable_autosave() | |
# yield desktop |
_unittest/conftest.py
Outdated
# If new_thread is set to false by a local_config, then don't close the desktop. | ||
# Intended for local debugging purposes only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# If new_thread is set to false by a local_config, then don't close the desktop. | |
# Intended for local debugging purposes only |
_unittest/conftest.py
Outdated
# if new_thread or os.name == "posix": | ||
# desktop.close_desktop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maxcapodi78 It is not required anymore to have this new-thread for local debugging session?
# if new_thread or os.name == "posix": | |
# desktop.close_desktop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
Now EDB tests and all tests that doesn't need AEDT runs before the others and outside the desktop