feat: Comments & logging cleaned up

Author: Bernard Szabo

tubular/scripts/dd_synthetic_tests.py

@@ -1,5 +1,4 @@
 #! /usr/bin/env python3
-from dataclasses import dataclass
 
 import click
 import logging
@@ -10,12 +9,12 @@
 
 class SyntheticTest:
     '''
-    Attributes for a specific synthetic test and its test run under a datadog CI batch
+    Attributes for a Datadog synthetic test and its test run
     '''
     def __init__(self, name, public_id):
-        self.name = name
-        self.public_id = public_id
-        self.test_run_id = None
+        self.name = name            # The test's Datadog name
+        self.public_id = public_id  # The test's Datadog Test ID
+        self.test_run_id = None     # The run ID given by Datadog to this test's invocation
         self.success = None
 
 class DatadogClient:
@@ -26,63 +25,68 @@ class DatadogClient:
 
     WAFFLE_SWITCH_TEST = SyntheticTest(
                 '''
-                Test on edx.org
+                "Waffle switch" test governing CI/CD synthetic testing
                 ''',
                 "sad-hqu-h33"
             )
 
     def __init__(self, api_key, app_key):
         self.api_key = api_key
         self.app_key = app_key
-        self.test_batch_id = None
-        self.trigger_time = None
-        self.tests_by_public_id = {}
+        self.test_batch_id = None   # A 'batch' is a set of tests intended to be run in parallel
+        self.trigger_time = None    # The system time at which a batch's execution was requested
+        self.tests_by_public_id = {} # Dictionary mapping Datadog test ID to all info we have for a specific test
 
     def trigger_synthetic_tests(self, tests_to_report: [SyntheticTest]):
         '''
-        Trigger running of one or more synthetic tests.
-        :param tests_to_report: List of tests to run and reported on
-        :return: None, but saves batch ID and test run IDs as object attributes
+        Trigger running of a batch of synthetic tests.
+        :param tests_to_report: List of tests to run and report on
+        :return: None, but saves test info including batch ID and test run IDs in 'self'
         '''
 
-        # Note that the list of tests to be run is actually one longer than the list of tests to be reported
-        # on. This is the so-called "waffle switch test". That test should be modified via the Datadog UI to either
-        # always pass or always fail, depending on whether synthetic testing is to be enabled at runtime or not.
-        # It's result affects how the pipeline operates, but is not treated as a reportable smoke test.
+        # Note that the list of tests to be run is one longer than the list of tests to be reported on.
+        # The extra test is the so-called "waffle switch test". That test should be modified via the Datadog UI
+        # to either always pass or always fail, depending on whether synthetic testing is to be enabled at runtime
+        # or not, respectively.
+        # While the test's result does affect how the pipeline operates, the result is not treated as reportable.
         tests_to_run = [self.WAFFLE_SWITCH_TEST] + tests_to_report
         self._record_requested_test_particulars(tests_to_run)
         self.trigger_time = time.time()  # Key timeouts off of this
         logging.info(f'CI batch triggered at time {self.trigger_time}')
 
         try:
-            response = self._trigger_batch_tests()
+            response = self._trigger_batch_tests()  # Kicks off asynchronous test execution for a batch of tests
             response_body = response.json()
-            self._record_batch_id(response_body)
-            self._map_test_run_ids(response_body)
+            self._record_batch_id(response_body)    # a single batch ID has now been assiged. Save for future reference
+            self._map_test_run_ids(response_body)   # one test run ID per test has been assigned. Save for reference.
 
         except Exception as e:
             raise Exception("Datadog error on triggering tests: " + str(e))
 
     def gate_on_waffle_switch(self):
         '''
         This is a bit hacky, but there's a designated test that's used as a waffle switch might be used in
-        a Django app. If the test passes, it means that the GoCD pipeline is to pass or fail per the result
-        of the Datadog synthetic tests that are run. If the test fails, then it means that the Datadog synthetic
-        test results are to be disregarded. When this is the case, the GoCD pipeline responsible for running the
-        tests should return with a summary success.
+        a Django app. If the test passes, it means that the synthetic testing GoCD pipeline is enabled, and the
+        build should only proceed if all reportable tests pass; if the test fails, the build should proceed irrespective
+        of any failures among the synthetic tests (which will be allowed to run, nonetheless). When this is intended,
+        the GoCD pipeline responsible for running the tests should just return a success code without waiting
+        for the reportable tests to complete their runs.
+
+        :return: Nothing, but terminates task with a success code if the synthetic testing feature is disabled
+        and logs the decision to skip testing on this build
         '''
-        logging.info("Gating on waffle switch")
         waffle_switch = self._poll_for_test_result(self.WAFFLE_SWITCH_TEST)
         if waffle_switch == False:
             switch_test_name = self.WAFFLE_SWITCH_TEST.name
             logging.warning(
-                f'*** Datadog Synthetic testing disabled via failing "Waffle flag" test {switch_test_name} ***')
+                f'*** Datadog Synthetic testing disabled via failing test {switch_test_name} ***')
             sys.exit(0)
 
     def get_and_record_test_results(self):
         '''
-        Poll for test results for all tests that were run, and save the results
-        in this datadog client object
+        Poll for pass/fail results for all batch tests
+
+        :return: Nothing, but saves pass/fail results in 'self'
         '''
         for test in list(self.tests_by_public_id.values()):
             test.success = self._poll_for_test_result(test)
@@ -170,7 +174,6 @@ def _poll_for_test_result(self, test):
 
         Returns None if still running; otherwise, returns True on test success and False on test failure.
         """
-        logging.info(f"Polling on test {test.public_id}: {test.name}, test_run_id = {test.test_run_id}")
         test_result = None
         while test_result is None and (time.time() - self.trigger_time) < (self.MAX_ALLOWABLE_TIME_SECS):
             time.sleep(5)  # Poll every 5 seconds