DEVPROD-8113: Define optional variable before referencing and add sup…

…port for mongo default variants (#22)

* fix for optional var

* changelog

* fix for wiredtiger

* fix tests

* update changelog

CHANGELOG.md

@@ -1,4 +1,7 @@
 # Changelog
+## 7.0.1 - 2024-06-11
+- Fix for unbound variable.
+- Add back support for mongo default variants
 
 ## 7.0.0 - 2024-06-10
 - `--display-variant-name` and `--build-variant` are `xor`'d together, instead of `or`'d.

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "git-co-evg-base"
-version = "7.0.0"
+version = "7.0.1"
 description = "Find a good commit to base your work on"
 authors = ["DevProd Services & Integrations Team <[email protected]>"]
 readme = "README.md"

src/goodbase/build_checker.py

@@ -23,27 +23,25 @@ class BuildChecks(BaseModel):
     active_tasks: Set of tasks that need to have run to use the build.
     """
 
-    build_variant_regex: Optional[List[str]] = []
-    display_name_regex: Optional[List[str]] = []
+    build_variant_regex: List[str]
+    display_name_regex: List[str]
     success_threshold: Optional[float] = None
     failure_threshold: Optional[float] = None
     run_threshold: Optional[float] = None
     successful_tasks: Optional[Set[str]] = None
     active_tasks: Optional[Set[str]] = None
 
-    def should_apply(self, build_variant: Optional[str], display_name: Optional[str]) -> bool:
+    def should_apply(self, build_variant: str, display_name: str) -> bool:
         """
         Check if the given build variant should apply to these checks.
 
         :param build_variant: Name of build variant to check.
         :param display_name: Display name of build variant to check.
         :return: True if these checks apply to the given build variant.
         """
-        if self.build_variant_regex and build_variant:
-            return any(re.match(bv_regex, build_variant) for bv_regex in self.build_variant_regex)
-        elif self.display_name_regex and display_name:
-            return any(re.match(dn_regex, display_name) for dn_regex in self.display_name_regex)
-        return False
+        return any(
+            re.match(bv_regex, build_variant) for bv_regex in self.build_variant_regex
+        ) or any(re.match(dn_regex, display_name) for dn_regex in self.display_name_regex)
 
     def check(self, build_status: BuildStatus) -> bool:
         """

src/goodbase/goodbase_cli.py

@@ -207,8 +207,8 @@ def display_criteria(self) -> None:
 
             for rule in group.rules:
                 table.add_row(
-                    "\n".join(rule.build_variant_regex) if rule.build_variant_regex else "",
-                    "\n".join(rule.display_name_regex) if rule.display_name_regex else "",
+                    "\n".join(rule.build_variant_regex),
+                    "\n".join(rule.display_name_regex),
                     f"{rule.success_threshold}" if rule.success_threshold else "",
                     f"{rule.run_threshold}" if rule.run_threshold else "",
                     "\n".join(rule.successful_tasks) if rule.successful_tasks else "",
@@ -342,8 +342,8 @@ def main(
     fail_threshold: float,
     evg_config_file: str,
     evg_project: str,
-    build_variant: Optional[List[str]],
-    display_variant_name: Optional[List[str]],
+    build_variant: List[str],
+    display_variant_name: List[str],
     commit_lookback: int,
     commit_limit: Optional[str],
     timeout_secs: Optional[int],
@@ -412,6 +412,15 @@ def main(
     evg_config_file = os.path.expanduser(evg_config_file)
     evg_api = RetryingEvergreenApi.get_api(config_file=evg_config_file)
 
+    if build_variant and display_variant_name:
+        click.echo(
+            click.style(
+                "Can only specify `--build-variant` or `--display-variant-name`, not both",
+                fg="red",
+            )
+        )
+        sys.exit(1)
+
     if branch:
         if git_operation == GitAction.NONE:
             git_operation = GitAction.CHECKOUT
@@ -439,10 +448,13 @@ def main(
         branch_name=branch,
         output_format=output_format,
     )
-    if display_variant_name:
-        display_variant_name_checks = display_variant_name
 
-    if ctx.get_parameter_source("display_variant_name") == ParameterSource.DEFAULT:
+    display_variant_name_checks = display_variant_name
+
+    if (
+        not build_variant
+        and ctx.get_parameter_source("display_variant_name") == ParameterSource.DEFAULT
+    ):
         for (
             project_regex,
             default_display_variant_name_regexes,
@@ -456,19 +468,9 @@ def main(
                 display_variant_name_checks = default_display_variant_name_regexes
                 break
 
-    if build_variant and display_variant_name:
-        click.echo(
-            click.style(
-                "Can only specify `--build-variant` or `--display-variant-name`, not both",
-                fg="red",
-            )
-        )
-        sys.exit(1)
-
-    if build_variant is not None:
-        build_checks = BuildChecks(build_variant_regex=build_variant)
-    if display_variant_name is not None:
-        build_checks = BuildChecks(display_variant_regex=display_variant_name_checks)
+    build_checks = BuildChecks(
+        build_variant_regex=build_variant, display_name_regex=display_variant_name_checks
+    )
 
     if pass_threshold is not None:
         build_checks.success_threshold = pass_threshold

tests/goodbase/services/test_evg_service.py

@@ -251,7 +251,9 @@ def test_no_build_meet_checks(self, evg_service):
             for i in range(n_builds)
         }
         mock_version = build_mock_version(mock_build_map)
-        build_checks = BuildChecks(build_variant_regex=[".*"], run_threshold=0.9)
+        build_checks = BuildChecks(
+            build_variant_regex=[".*"], display_name_regex=[], run_threshold=0.9
+        )
 
         result = evg_service.check_version(mock_version, [build_checks])
 
@@ -268,7 +270,9 @@ def test_all_build_meet_checks(self, evg_service):
             for i in range(n_builds)
         }
         mock_version = build_mock_version(mock_build_map)
-        build_checks = BuildChecks(build_variant_regex=[".*"], run_threshold=0.9)
+        build_checks = BuildChecks(
+            build_variant_regex=[".*"], display_name_regex=[], run_threshold=0.9
+        )
 
         result = evg_service.check_version(mock_version, [build_checks])
 
@@ -287,6 +291,7 @@ def test_all_build_meet_checks_with_failure_threshold(self, evg_service):
         mock_version = build_mock_version(mock_build_map)
         build_checks = BuildChecks(
             build_variant_regex=[".*"],
+            display_name_regex=[],
             run_threshold=0.9,
             failure_threshold=0.1,
         )
@@ -313,6 +318,7 @@ def test_one_build_meet_checks_with_failure_threshold(self, evg_service):
         mock_version = build_mock_version(mock_build_map)
         build_checks = BuildChecks(
             build_variant_regex=[".*"],
+            display_name_regex=[],
             run_threshold=0.9,
             failure_threshold=0.1,
         )
@@ -333,7 +339,7 @@ def test_some_build_meet_checks(self, evg_service):
         }
         mock_version = build_mock_version(mock_build_map)
         build_checks = BuildChecks(
-            build_variant_regex=[".*"], display_name_regex=None, run_threshold=0.9
+            build_variant_regex=[".*"], display_name_regex=[], run_threshold=0.9
         )
 
         result = evg_service.check_version(mock_version, [build_checks])
@@ -351,7 +357,9 @@ def test_some_build_meet_checks_but_are_filtered_out(self, evg_service):
             for i in range(n_builds)
         }
         mock_version = build_mock_version(mock_build_map)
-        build_checks = BuildChecks(build_variant_regex=["^build_0$"], run_threshold=0.9)
+        build_checks = BuildChecks(
+            build_variant_regex=["^build_0$"], display_name_regex=[], run_threshold=0.9
+        )
 
         result = evg_service.check_version(mock_version, [build_checks])