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.

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

Merged
merged 1 commit into from
Jan 11, 2024
Merged

Add utils tests #413

merged 1 commit into from
Jan 11, 2024

Conversation

janbjorge
Copy link
Contributor

@janbjorge janbjorge commented Jan 10, 2024

Test coverage hunting: ~ 14% more coverage in the src/fmu/dataio/_utils.py

Before:

(dataio) ➜  fmu-dataio git:(main) coverage report --include=src/fmu/dataio/_utils.py 
Name                       Stmts   Miss  Cover
----------------------------------------------
src/fmu/dataio/_utils.py     226     45    80%
----------------------------------------------
TOTAL                        226     45    80%

After:

(dataio) ➜  fmu-dataio git:(more-utils-tests) ✗ coverage report --include=src/fmu/dataio/_utils.py
Name                       Stmts   Miss  Cover
----------------------------------------------
src/fmu/dataio/_utils.py     218     12    94%
----------------------------------------------
TOTAL                        218     12    94%

@janbjorge janbjorge self-assigned this Jan 10, 2024
def inside_rms(func):
"""Decorator for being inside RMS"""

@wraps(func)
def wrapper(*args, **kwargs):
ExportData._inside_rms = True
retval = func(*args, **kwargs)
Copy link
Contributor Author

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.

@janbjorge janbjorge added enhancement New feature or request and removed enhancement New feature or request labels Jan 10, 2024
@janbjorge janbjorge requested review from mferrera, perolavsvendsen and tnatt and removed request for mferrera January 10, 2024 17:20
@janbjorge janbjorge marked this pull request as ready for review January 10, 2024 17:21
Copy link
Collaborator

@mferrera mferrera left a 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
Copy link
Collaborator

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?

Comment on lines +61 to +62
unset = object()
old = os.environ.get("INSIDE_RMS", unset)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! For uniqueness?

Copy link
Contributor Author

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.

Comment on lines 92 to 95
assert utils.get_object_name(Polygons()) is None
assert utils.get_object_name(Polygons(name="Not poly")) == "Not poly"
Copy link
Collaborator

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

@janbjorge janbjorge merged commit 55cb36f into equinor:main Jan 11, 2024
11 checks passed
@janbjorge janbjorge deleted the more-utils-tests branch January 11, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants