coverage, PR feedback

cumulus_library/actions/builder.py

@@ -178,8 +178,8 @@ def _run_workflow(
                 config=workflow_config,
                 data_path=manifest.data_path / f"{manifest.get_study_prefix()}/valueset",
             )
-        case _:
-            raise errors.StudyManifestParsingError(  # pragma: no cover
+        case _:  # pragma: no cover
+            raise errors.StudyManifestParsingError(
                 f"{toml_path} references an invalid workflow type {config_type}."
             )
     builder.execute_queries(
@@ -201,7 +201,7 @@ def build_matching_files(
     config: base_utils.StudyConfig,
     manifest: study_manifest.StudyManifest,
     *,
-    builder: str,
+    builder: str | None,
     db_parser: databases.DatabaseParser = None,
 ):
     """targets all table builders matching a target string for running
@@ -212,9 +212,12 @@ def build_matching_files(
     :keyword db_parser: an object implementing DatabaseParser for the target database"""
     all_generators = manifest.get_all_generators()
     matches = []
-    for file in all_generators:
-        if builder and file.find(builder) != -1:
-            matches.append(file)
+    if not builder:  # pragma: no cover
+        matches = all_generators
+    else:
+        for file in all_generators:
+            if file.find(builder) != -1:
+                matches.append(file)
     build_study(config, manifest, db_parser=db_parser, file_list=matches)
 
 
@@ -257,8 +260,6 @@ def _run_raw_queries(
     queries = []
     for query in base_utils.parse_sql(base_utils.load_text(f"{manifest._study_path}/{filename}")):
         queries.append([query, filename])
-    if len(queries) == 0:
-        return []
     for query in queries:
         query[0] = base_utils.update_query_if_schema_specified(query[0], manifest)
         query[0] = query[0].replace(

cumulus_library/builders/protected_table_builder.py

@@ -67,11 +67,14 @@ def prepare_queries(
                 TRANSACTION_COLS_TYPES,
             )
         )
-        files = manifest.get_file_list()
-        files = [file for file in files if file.endswith(".toml")]
+        files = manifest.get_all_workflows()
         if len(files) == 0:
             return
         stats_types = set(item.value for item in enums.StatisticsTypes)
+        # In this loop, we are just checking to see if :any: workflow is a stats
+        # type workflow - if so, we'll create a table to hold data of stats runs
+        # (if it doesn't already exist) outside of the study lifecycle for
+        # persistence reasons
         for file in files:
             toml_path = pathlib.Path(f"{manifest._study_path}/{file}")
             with open(toml_path, "rb") as file:

cumulus_library/studies/core/manifest.toml

@@ -1,6 +1,6 @@
 study_prefix = "core"
 
-[table_builder_config]
+[file_config]
 file_names = [
     "builder_prereq_tables.py",
     "builder_allergyintolerance.py",
@@ -9,17 +9,9 @@ file_names = [
     "builder_encounter.py",
     "builder_documentreference.py",
     "builder_medicationrequest.py",
-    "builder_observation.py"
-]
-
-[sql_config]
-file_names = [
+    "builder_observation.py",
     "observation_type.sql",
     "meta_date.sql",
-]
-
-[counts_builder_config]
-file_names = [
     "count_core.py"
 ]
 

cumulus_library/studies/discovery/manifest.toml

@@ -1,6 +1,6 @@
 study_prefix = "discovery"
 
-[table_builder_config]
+[file_config]
 file_names = [
     "code_detection.py",
 ]

cumulus_library/study_manifest.py

@@ -123,7 +123,7 @@ def get_sql_file_list(self, continue_from: str | None = None) -> list[str] | Non
         """
         sql_config = self._study_config.get("sql_config", {})
         sql_files = sql_config.get("file_names", []) or []
-        if continue_from:
+        if continue_from:  # pragma: no cover
             for pos, file in enumerate(sql_files):
                 if continue_from.replace(".sql", "") == file.replace(".sql", ""):
                     sql_files = sql_files[pos:]
@@ -204,15 +204,18 @@ def get_export_table_list(self) -> list[ManifestExport] | None:
                 found_name.add(export.name)
         return export_table_list
 
-    def get_all_generators(self) -> list[str]:
-        """Convenience method for getting files that generate sql queries"""
+    def get_all_files(self, file_type: str):
+        """Convenience method for getting files of a type from a manifest"""
         files = self.get_file_list()
-        return [file for file in files if file.endswith(".py")]
+        return [file for file in files if file.endswith(file_type)]
+
+    def get_all_generators(self) -> list[str]:
+        """Convenience method for getting builder-based files"""
+        return self.get_all_files(".py")
 
     def get_all_workflows(self) -> list[str]:
-        """Convenience method for getting config files"""
-        files = self.get_file_list()
-        return [file for file in files if file.endswith(".toml")]
+        """Convenience method for getting workflow config files"""
+        return self.get_all_files(".toml")
 
     def get_prefix_with_seperator(self) -> str:
         """Convenience method for getting the appropriate prefix for tables"""

tests/test_cli.py

@@ -426,6 +426,24 @@ def test_clean(tmp_path, args, expected, raises):
             2,
             pytest.raises(errors.StudyManifestParsingError),
         ),
+        (
+            [
+                "build",
+                "-t",
+                "study_invalid_unsupported_file",
+                "-s",
+                "tests/test_data/study_invalid_unsupported_file/",
+            ],
+            [
+                "export",
+                "-t",
+                "study_invalid_unsupported_file",
+                "-s",
+                "tests/test_data/study_invalid_unsupported_file/",
+            ],
+            2,
+            pytest.raises(errors.StudyManifestParsingError),
+        ),
     ],
 )
 def test_cli_executes_queries(tmp_path, build_args, export_args, expected_tables, raises):

tests/test_data/study_invalid_unsupported_file/manifest.toml

@@ -0,0 +1,7 @@
+study_prefix = "study_invalid_unsupported_file"
+
+[file_config]
+file_names = ["test.txt"]
+
+[export_config]
+export_list = ["study_invalid_unsupported_file__table"]

tests/test_data/study_invalid_unsupported_file/test.txt

@@ -0,0 +1 @@
+This should not be a study file