-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add utils tests #413
Add utils tests #413
Conversation
def inside_rms(func): | ||
"""Decorator for being inside RMS""" | ||
|
||
@wraps(func) | ||
def wrapper(*args, **kwargs): | ||
ExportData._inside_rms = True | ||
retval = func(*args, **kwargs) |
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 func raises, revert to old state never happens. In my case, got a few tailing tests that failed.
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.
Super. Some comments for your consideration
@@ -45,15 +46,39 @@ def pytest_configure(): | |||
cprint(80 * "=", "red", attrs=["blink"]) | |||
|
|||
|
|||
@contextlib.contextmanager |
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.
Would it be reasonable to expose these context managers through fixtures?
unset = object() | ||
old = os.environ.get("INSIDE_RMS", unset) |
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.
Interesting! For uniqueness?
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.
Yeah, just in case its set to None
. If i do not use a sentinel value i want be able do differentiate between the two.
tests/test_units/test_utils.py
Outdated
assert utils.get_object_name(Polygons()) is None | ||
assert utils.get_object_name(Polygons(name="Not poly")) == "Not poly" |
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.
Could be worth explicitly testing with Polygons(name="poly")
is None as well since this seems like a default we could inadvertently change in the future without thinking it's relied upon
Test coverage hunting: ~ 14% more coverage in the src/fmu/dataio/_utils.py
Before:
After: