Repository: great-expectations/great_expectations
Pull request #6778 (2023-01-12 14:12:21)

[MAINTENANCE] Refactor `file_relative_path` util
Changes proposed in this pull request:
- Use `pathlib`
- Make the method test strict existence


### Definition of Done
- [x] My code follows the Great Expectations [style guide](https://docs.greatexpectations.io/docs/contributing/style_guides/code_style)
- [x] I have performed a [self-review](https://docs.greatexpectations.io/docs/contributing/contributing_checklist) of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added [unit tests](https://docs.greatexpectations.io/docs/contributing/contributing_test#writing-unit-and-integration-tests) where applicable and made sure that new and existing tests are passing.
- [x] I have run any local integration tests and made sure that nothing is broken.


codesee-maps[bot]: <details>
<summary><b>👇 <a href="https://app.codesee.io/r/reviews?pr=6778&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations">Click on the image</a> for a new way to code review</b>
</summary>

  - Make big changes easier — review code in small groups of related files
  - Know where to start — see the whole change at a glance
  - Take a code tour — explore the change with an interactive tour
  - Make comments and review — all fully sync’ed with github

    [Try it now!](https://app.codesee.io/r/reviews?pr=6778&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations)
</details>

[![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/great-expectations/great_expectations/6778/3fb9f757/e4e2f458bf5549ba6433092418527be414746b77.svg)](https://app.codesee.io/r/reviews?pr=6778&src=https%3A%2F%2Fgithub.com%2Fgreat-expectations%2Fgreat_expectations)

#### Legend
![CodeSee Map Legend](https://app.codesee.io/Legend_v2.png)
Kilo59: Why close? It looked like it only failing because a couple of tests were using it to point to files that didn't exist. Which could be resolved by changing the strict default to false or just passing as false for those cases. 

Patch:
diff --git a/great_expectations/data_context/util.py b/great_expectations/data_context/util.py
index 1c240cfac..bd5a1388d 100644
--- a/great_expectations/data_context/util.py
+++ b/great_expectations/data_context/util.py
@@ -1,13 +1,14 @@
 import copy
 import inspect
 import logging
-import os
+import pathlib
 import warnings
 from typing import Any, Optional
 from urllib.parse import urlparse
 
 import pyparsing as pp
 
+from great_expectations.alias_types import PathStr
 from great_expectations.types import safe_deep_copy
 from great_expectations.util import load_class, verify_dynamic_loading_support
 
@@ -106,7 +107,11 @@ def format_dict_for_error_message(dict_):
     return "\n\t".join("\t\t".join((str(key), str(dict_[key]))) for key in dict_)
 
 
-def file_relative_path(dunderfile, relative_path):
+def file_relative_path(
+    source_path: PathStr,
+    relative_path: PathStr,
+    strict: bool = True,
+) -> str:
     """
     This function is useful when one needs to load a file that is
     relative to the position of the current file. (Such as when
@@ -116,9 +121,12 @@ def file_relative_path(dunderfile, relative_path):
     It is meant to be used like the following:
     file_relative_path(__file__, 'path/relative/to/file')
 
+    This has been modified from Dagster's utils:
     H/T https://github.com/dagster-io/dagster/blob/8a250e9619a49e8bff8e9aa7435df89c2d2ea039/python_modules/dagster/dagster/utils/__init__.py#L34
     """
-    return os.path.join(os.path.dirname(dunderfile), relative_path)
+    dir_path = pathlib.Path(source_path).parent
+    abs_path = dir_path.joinpath(relative_path).resolve(strict=strict)
+    return str(abs_path)
 
 
 def parse_substitution_variable(substitution_variable: str) -> Optional[str]:


Test patch:
diff --git a/tests/render/test_column_section_renderer.py b/tests/render/test_column_section_renderer.py
index 8e93f2c95..c8e5020f9 100644
--- a/tests/render/test_column_section_renderer.py
+++ b/tests/render/test_column_section_renderer.py
@@ -92,6 +92,7 @@ def test_render_profiling_results_column_section_renderer(titanic_validation_res
                 "./output/test_render_profiling_results_column_section_renderer__"
                 + column
                 + ".json",
+                strict=False,
             ),
             "w",
         ) as outfile:
@@ -126,6 +127,7 @@ def test_render_expectation_suite_column_section_renderer(titanic_expectations):
                 "./output/test_render_expectation_suite_column_section_renderer"
                 + column
                 + ".json",
+                strict=False,
             ),
             "w",
         ) as outfile:
diff --git a/tests/render/test_data_documentation_site_builder.py b/tests/render/test_data_documentation_site_builder.py
index c667648eb..defa9d2cf 100644
--- a/tests/render/test_data_documentation_site_builder.py
+++ b/tests/render/test_data_documentation_site_builder.py
@@ -654,7 +654,7 @@ def test_site_builder_usage_statistics_enabled(
         },
     )
     site_builder_return_obj = site_builder.build()
-    index_page_path = site_builder_return_obj[0]
+    index_page_path = site_builder_return_obj[0][7:]  # strip prefix "file:///"
     links_dict = site_builder_return_obj[1]
     expectation_suite_pages = [
         file_relative_path(index_page_path, expectation_suite_link_dict["filepath"])
@@ -672,7 +672,7 @@ def test_site_builder_usage_statistics_enabled(
     expected_logo_url = "https://great-expectations-web-assets.s3.us-east-2.amazonaws.com/logo-long.png?d=20190924T231836.000000Z&dataContextId=f43d4897-385f-4366-82b0-1a8eda2bf79c"
 
     for page_path in page_paths_to_check:
-        with open(page_path[7:]) as f:
+        with open(page_path) as f:
             page_contents = f.read()
             assert expected_logo_url in page_contents
 
@@ -705,7 +705,7 @@ def test_site_builder_usage_statistics_disabled(
         },
     )
     site_builder_return_obj = site_builder.build()
-    index_page_path = site_builder_return_obj[0]
+    index_page_path = site_builder_return_obj[0][7:]  # strip prefix "file:///"
     links_dict = site_builder_return_obj[1]
     expectation_suite_pages = [
         file_relative_path(index_page_path, expectation_suite_link_dict["filepath"])
@@ -723,7 +723,7 @@ def test_site_builder_usage_statistics_disabled(
     expected_logo_url = "https://great-expectations-web-assets.s3.us-east-2.amazonaws.com/logo-long.png?d=20190924T231836.000000Z"
 
     for page_path in page_paths_to_check:
-        with open(page_path[7:]) as f:
+        with open(page_path) as f:
             page_contents = f.read()
             assert expected_logo_url in page_contents
             assert data_context_id not in page_contents
diff --git a/tests/render/test_default_jinja_view.py b/tests/render/test_default_jinja_view.py
index 21b1a72a9..091d8b303 100644
--- a/tests/render/test_default_jinja_view.py
+++ b/tests/render/test_default_jinja_view.py
@@ -51,7 +51,9 @@ def test_render_DefaultJinjaPageView_meta_info():
     html = DefaultJinjaPageView().render(document)
     with open(
         file_relative_path(
-            __file__, "./output/test_render_DefaultJinjaPageView_meta_info.html"
+            __file__,
+            "./output/test_render_DefaultJinjaPageView_meta_info.html",
+            strict=False,
         ),
         "w",
     ) as outfile:
diff --git a/tests/render/test_render.py b/tests/render/test_render.py
index 2b9ed562c..4d4edbcc4 100644
--- a/tests/render/test_render.py
+++ b/tests/render/test_render.py
@@ -97,7 +97,9 @@ def test_smoke_render_profiling_results_page_renderer(titanic_profiled_evrs_1):
     rendered = ProfilingResultsPageRenderer().render(titanic_profiled_evrs_1)
     with open(
         file_relative_path(
-            __file__, "./output/test_render_profiling_results_page_renderer.json"
+            __file__,
+            "./output/test_render_profiling_results_page_renderer.json",
+            strict=False,
         ),
         "w",
     ) as outfile:
@@ -127,6 +129,7 @@ def test_render_profiling_results_column_section_renderer(titanic_profiled_evrs_
                 "./output/test_render_profiling_results_column_section_renderer__"
                 + column
                 + ".json",
+                strict=False,
             ),
             "w",
         ) as outfile:
@@ -145,7 +148,9 @@ def test_smoke_render_validation_results_page_renderer(titanic_profiler_evrs):
     rendered = ValidationResultsPageRenderer().render(titanic_profiler_evrs)
     with open(
         file_relative_path(
-            __file__, "./output/test_render_validation_results_page_renderer.json"
+            __file__,
+            "./output/test_render_validation_results_page_renderer.json",
+            strict=False,
         ),
         "w",
     ) as outfile:
@@ -174,6 +179,7 @@ def test_render_validation_results_column_section_renderer(titanic_profiler_evrs
                 "./output/test_render_validation_results_column_section_renderer__"
                 + column
                 + ".json",
+                strict=False,
             ),
             "w",
         ) as outfile:
@@ -210,6 +216,7 @@ def test_render_expectation_suite_column_section_renderer(
                 "./output/test_render_expectation_suite_column_section_renderer"
                 + column
                 + ".json",
+                strict=False,
             ),
             "w",
         ) as outfile:
@@ -291,7 +298,9 @@ def test_render_profiled_fixture_expectation_suite(
 
     with open(
         file_relative_path(
-            __file__, "./output/test_render_profiled_fixture_expectation_suite.html"
+            __file__,
+            "./output/test_render_profiled_fixture_expectation_suite.html",
+            strict=False,
         ),
         "wb",
     ) as f:
@@ -316,6 +325,7 @@ def test_render_profiled_fixture_expectation_suite_with_distribution(
         file_relative_path(
             __file__,
             "./output/titanic_dataset_profiler_expectation_suite_with_distribution.html",
+            strict=False,
         ),
         "wb",
     ) as f:
@@ -332,7 +342,9 @@ def test_render_profiling_results(titanic_profiled_evrs_1):
     rendered_page = DefaultJinjaPageView().render(rendered_content)
 
     with open(
-        file_relative_path(__file__, "./output/test_render_profiling_results.html"),
+        file_relative_path(
+            __file__, "./output/test_render_profiling_results.html", strict=False
+        ),
         "wb",
     ) as f:
         f.write(rendered_page.encode("utf-8"))
@@ -348,7 +360,9 @@ def test_render_validation_results(titanic_profiled_evrs_1):
     rendered_page = DefaultJinjaPageView().render(rendered_content)
 
     with open(
-        file_relative_path(__file__, "./output/test_render_validation_results.html"),
+        file_relative_path(
+            __file__, "./output/test_render_validation_results.html", strict=False
+        ),
         "wb",
     ) as f:
         f.write(rendered_page.encode("utf-8"))
@@ -378,6 +392,7 @@ def test_smoke_render_profiling_results_page_renderer_with_exception(
         file_relative_path(
             __file__,
             "./output/test_render_profiling_results_column_section_renderer_with_exception.html",
+            strict=False,
         ),
         "wb",
     ) as outfile:
@@ -400,7 +415,8 @@ def test_full_oobe_flow():
     rendered_page = DefaultJinjaPageView().render(rendered_content)
 
     with open(
-        file_relative_path(__file__, "./output/test_full_oobe_flow.html"), "wb"
+        file_relative_path(__file__, "./output/test_full_oobe_flow.html", strict=False),
+        "wb",
     ) as f:
         f.write(rendered_page.encode("utf-8"))
 
diff --git a/tests/render/test_render_BulletListContentBlock.py b/tests/render/test_render_BulletListContentBlock.py
index bc8a4e9a3..4719dcb97 100644
--- a/tests/render/test_render_BulletListContentBlock.py
+++ b/tests/render/test_render_BulletListContentBlock.py
@@ -145,7 +145,9 @@ def test_all_expectations_using_test_definitions():
 
     with open(
         file_relative_path(
-            __file__, "./output/test_render_bullet_list_content_block.json"
+            __file__,
+            "./output/test_render_bullet_list_content_block.json",
+            strict=False,
         ),
         "w",
     ) as f: