Added extensive comments to the aggregator's unit test

src/service/clmcservice/aggregation/test_aggregator.py

@@ -21,21 +21,34 @@
 ##      Created Date :          04-06-2018
 ##      Created for Project :   FLAME
 """
-from collections import OrderedDict
+
 from threading import Event
 from unittest import mock
-from time import sleep
 from clmcservice.aggregation.aggregator import AggregatorThread
 from clmcservice.aggregation.influx_data_interface import MockResultSet, network_result_item, service_result_item, drop_timestamp, generate_e2e_no_timestamp_row
 
 
 class TestAggregation(object):
+    """
+    A unit test to ensure the functionality of the aggregator is correct.
+    """
 
-    ACTUAL_RESULTS = "actual_aggregated_results"
-    EXPECTED_RESULTS = "expected_aggregated_results"
-    FINISHED = "finished_event"
+    ACTUAL_RESULTS = "actual_aggregated_results"  # the attribute name of the actual results data structure
+    EXPECTED_RESULTS = "expected_aggregated_results"  # the attribute name of the expected results data structure
+    FINISHED = "finished_event"  # the attribute name of the flag object, which marks the end of the test
 
     def points_generator(self, network_items, service_items):
+        """
+        A generator method intended to be used by the mock db client when involving the mocked query() method. It takes the network and service items, and generates a result from
+        those items each time query() is called by taking turns - starts with network result, followed by service result and then it repeats, until all items have been exhausted.
+        Network items and service items are expected to have the same length.
+
+        :param network_items: the network data to generate from
+        :param service_items: the service data to generate from
+
+        :return: a generator object
+        """
+
         assert len(network_items) == len(service_items), "The data points generator must receive the same number of network items as the number of service items"
         index = 0
 
@@ -47,6 +60,7 @@ class TestAggregation(object):
 
             # before yielding the service data points, check if both sets of data points are enumerated
             if index == len(network_items)-1:
+                # if so, set the finished flag of the test
                 getattr(self, self.FINISHED).set()
 
             yield MockResultSet(items)
@@ -54,7 +68,14 @@ class TestAggregation(object):
             index += 1
 
     def setup_mock_db_client(self, mock_class):
-        setattr(self, self.ACTUAL_RESULTS, [])
+        """
+        Sets up a mock db client and also defines the expected aggregation results from the test.
+
+        :param mock_class: the mock class used as an influx db client instance
+        :return:
+        """
+
+        setattr(self, self.ACTUAL_RESULTS, [])  # initially, there are no actual results, these are built progressively while the aggregator is running
         setattr(self, self.EXPECTED_RESULTS, [
             generate_e2e_no_timestamp_row(path_id="SR1-SR3", source_sfr="SR1", target_sfr="SR3", endpoint="endpoint1", sf_instance="ms1.flame.org", delay_forward=10,
                                           delay_reverse=15, delay_service=10, avg_request_size=1024, avg_response_size=8, avg_bandwidth=1200),
@@ -74,10 +95,13 @@ class TestAggregation(object):
                                           delay_reverse=27, delay_service=105, avg_request_size=1024, avg_response_size=128, avg_bandwidth=1200),
             generate_e2e_no_timestamp_row(path_id="SR14-SR15", source_sfr="SR15", target_sfr="SR14", endpoint="endpoint14", sf_instance="ms1.flame.org", delay_forward=27,
                                           delay_reverse=24, delay_service=85, avg_request_size=32, avg_response_size=64, avg_bandwidth=1200),
-        ])
+        ])  # defines the expected rows from the aggregation
         setattr(self, self.FINISHED, Event())
 
+        # initialises the influx data generator, which is involved each time the query() method of the mock db client is called
         mock_points = self.points_generator(
+            # network items is a list of tuples, each tuple represents a result from a query; each time query() is called and a network measurement must be generated, then one of
+            # these tuples is generated, empty tuple means result with no points
             network_items=[
                 (
                     network_result_item("network_delays", "SR1-SR3", "SR1", "SR3", 10, 1200),
@@ -115,6 +139,8 @@ class TestAggregation(object):
                     network_result_item("network_delays", "SR14-SR15", "SR15", "SR14", 27, 1200),
                 )
             ],
+            # service items is a list of tuples, each tuple represents a result from a query; each time query() is called and a service measurement must be generated, then one of
+            # these tuples is generated, empty tuple means result with no points
             service_items=[
                 (
                     service_result_item("service_delays", "SR3", "endpoint1", "ms1.flame.org", 10, 1024, 8),
@@ -144,25 +170,42 @@ class TestAggregation(object):
             ]
         )
 
-        mock_class.query = lambda query: next(mock_points)
-        mock_class.write_points = lambda points: getattr(self, self.ACTUAL_RESULTS).append(drop_timestamp(points[0]))
+        # implement the query() and write_points() methods of the mock db client
+        mock_class.query = lambda query: next(mock_points)  # query() returns the next element of the mock_points generator
+        mock_class.write_points = lambda points: getattr(self, self.ACTUAL_RESULTS).append(drop_timestamp(points[0]))  # write_points() adds aggregated rows to actual results list
+
+        # in the end of the test, we can compare the expected results with the actual results that were generated during the aggregation process
 
     @mock.patch('clmcservice.aggregation.aggregator.InfluxDBClient', autospec=True)
     def test_aggregator(self, MockDBClient):
+        """
+        The actual test that's executed when running pytest.
+
+        :param MockDBClient: a mock object argument passed by the mock.patch decorator. The decorator changes all occurrences of InfluxDBClient in the aggregator's code to the
+        return value of this MockDBClient object
+        """
+
+        # set up the mock db client by providing implementations for the necessary methods (query and write_points)
         self.setup_mock_db_client(MockDBClient.return_value)
 
+        # start the aggregator as a thread, report period set to 2 so that the unit tests is not taking too long
         t = AggregatorThread(report_period=2)
         t.start()
 
-        while not getattr(self, self.FINISHED).is_set():
-            sleep(1)
+        # wait until the finished flag has been set
+        getattr(self, self.FINISHED).wait()
 
+        # stop the thread when the aggregation has finished
         t.stop()
 
+        # compare the expected results with teh actual results that were collected during the aggregation process
         expected_results = getattr(self, self.EXPECTED_RESULTS)
         actual_results = getattr(self, self.ACTUAL_RESULTS)
         assert type(actual_results) is list
         assert type(expected_results) is list
         assert len(actual_results) == len(expected_results), "Actual and expected result differ in length."
-        assert sorted(actual_results, key=lambda k: k['tags']['path_ID'])  == sorted(expected_results, key=lambda k: k['tags']['path_ID']), \
+
+        # we compare sorted versions of the expected and actual results; this is because the aggregator implementation uses dictionary for efficiency purposes, hence the order of
+        # the collected results may vary, especially on different OS; hence, we only care about the two list of results to contain the same elements
+        assert sorted(actual_results, key=lambda k: k['tags']['path_ID']) == sorted(expected_results, key=lambda k: k['tags']['path_ID']), \
             "Test failure - aggregation process returns incorrect results."