From be601b7cfe542358f954e180b61b02c98fcfb27b Mon Sep 17 00:00:00 2001
From: Troy Dai <troy.dai@outlook.com>
Date: Thu, 2 Aug 2018 16:12:22 -0700
Subject: [PATCH] Make telemetry be compatible with Python 2 (#6959)

* Make telemetry be compatible with Python 2
* Add unit test stage
---
 .gitignore                                       |  3 +++
 .travis.yml                                      | 15 ++++++++++++++-
 scripts/ci/unittest.sh                           |  8 ++++++++
 .../telemetry/components/records_collection.py   |  2 +-
 .../telemetry/components/telemetry_logging.py    |  8 +++++---
 .../cli/telemetry/components/telemetry_note.py   | 16 ++++++----------
 .../cli/telemetry/tests/test_telemetry_client.py |  8 ++++----
 .../telemetry/tests/test_telemetry_logging.py    | 10 ++++++----
 .../cli/telemetry/tests/test_telemetry_note.py   | 10 +++++++---
 .../azure/cli/telemetry/util.py                  |  3 ++-
 src/azure-cli-telemetry/setup.cfg                |  3 ---
 src/azure-cli-telemetry/setup.py                 |  4 +---
 src/azure-cli-telemetry/tox.ini                  |  8 ++++++++
 13 files changed, 65 insertions(+), 33 deletions(-)
 create mode 100755 scripts/ci/unittest.sh
 create mode 100644 src/azure-cli-telemetry/tox.ini

diff --git a/.gitignore b/.gitignore
index 86f4ed4b72f..260468492dc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -105,5 +105,8 @@ pip.log
 # Setuptools
 .eggs/
 
+# Tox
+.tox/
+
 az_command_coverage.txt
 
diff --git a/.travis.yml b/.travis.yml
index 32d7ec72821..b4b330c1979 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -11,7 +11,7 @@ addons:
       - libssl-dev
       - libffi-dev
       - python-dev
-install: echo 'skip'
+install: pip install tox
 cache: pip
 jobs:
   include:
@@ -31,6 +31,14 @@ jobs:
       env: PURPOSE='Lint Command Table and Help'
       script: ./scripts/ci/verify_linter.sh
       python: 3.6
+    - stage: unittest
+      python: 3.6
+      env: TOXENV=py36
+      script: ./scripts/ci/unittest.sh
+    - stage: unittest
+      python: 2.7
+      env: TOXENV=py27
+      script: ./scripts/ci/unittest.sh
     - stage: verify
       env: PURPOSE='Automation'
       script: ./scripts/ci/test_automation.sh
@@ -73,3 +81,8 @@ jobs:
       python: 3.6
       env: PURPOSE='Automation Docker'
       if: repo = Azure/azure-cli and type = push
+stages:
+  - precheck
+  - unittest
+  - verify
+  - publish
diff --git a/scripts/ci/unittest.sh b/scripts/ci/unittest.sh
new file mode 100755
index 00000000000..cf22d9bff6f
--- /dev/null
+++ b/scripts/ci/unittest.sh
@@ -0,0 +1,8 @@
+#!/usr/bin/env bash
+
+set -e
+
+for TOX_FILE in $(find src -name tox.ini); do
+    pwd
+    (cd $(dirname $TOX_FILE) && tox)
+done
\ No newline at end of file
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/components/records_collection.py b/src/azure-cli-telemetry/azure/cli/telemetry/components/records_collection.py
index edac590df07..43c41d78717 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/components/records_collection.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/components/records_collection.py
@@ -80,7 +80,7 @@ def _read_file(self, path):
     def _add_record(self, content_line):
         """ Parse a line in the recording file. """
         try:
-            time, content = content_line.split(',', maxsplit=1)
+            time, content = content_line.split(',', 1)
             time = datetime.datetime.strptime(time, '%Y-%m-%dT%H:%M:%S')
             if time > self._last_sent:
                 self._next_send = max(self._next_send, time)
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_logging.py b/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_logging.py
index 160bfbd302b..09bc77f20c2 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_logging.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_logging.py
@@ -26,9 +26,11 @@ def config_logging_for_upload(config_dir):
     if folder:
         handler = logging.handlers.RotatingFileHandler(os.path.join(folder, TELEMETRY_LOG_NAME),
                                                        maxBytes=10 * 1024 * 1024, backupCount=5)
-        logging.basicConfig(handlers=[handler],
-                            level=logging.DEBUG,
-                            format='%(process)d : %(asctime)s : %(levelname)s : %(name)s : %(message)s')
+        del logging.root.handlers[:]
+        formatter = logging.Formatter('%(process)d : %(asctime)s : %(levelname)s : %(name)s : %(message)s', None)
+        handler.setFormatter(formatter)
+        logging.root.addHandler(handler)
+        logging.root.setLevel(logging.DEBUG)
 
 
 def get_logger(section_name):
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_note.py b/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_note.py
index 22200443d79..ed461b6214b 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_note.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/components/telemetry_note.py
@@ -4,6 +4,7 @@
 # --------------------------------------------------------------------------------------------
 
 import os
+import time
 import datetime
 
 import portalocker.utils
@@ -17,7 +18,7 @@ def __init__(self, config_dir):
         self._logger = get_logger('note')
 
         if not os.path.exists(self._path):
-            super(TelemetryNote, self).__init__(self._path, mode='x', timeout=0.1, fail_when_locked=True)
+            super(TelemetryNote, self).__init__(self._path, mode='w', timeout=0.1, fail_when_locked=True)
         else:
             super(TelemetryNote, self).__init__(self._path, mode='r+', timeout=1, fail_when_locked=True)
 
@@ -36,19 +37,14 @@ def get_last_sent(self):
         fallback = datetime.datetime.min
 
         try:
-            if not self.fh.readable():
-                self._logger.debug('The file is not readable. Fallback to datetime.min.')
-                return fallback
-
             raw = self.fh.read().strip()
             last_send = datetime.datetime.strptime(raw, '%Y-%m-%dT%H:%M:%S')
             self._logger.info("Read timestamp from the note. The last send was %s.", last_send)
-        except (ValueError, IOError) as err:
-            last_send = datetime.datetime.min
+            return last_send
+        except (AttributeError, ValueError, IOError) as err:
             self._logger.warning("Fail to parse or read the timestamp '%s' in the note file. Set the last send time "
                                  "to minimal. Reason: %s", raw, err)
-
-        return last_send
+            return fallback
 
     def update_telemetry_note(self, next_send):
         # note update retry
@@ -69,7 +65,7 @@ def update_telemetry_note(self, next_send):
 
     def touch(self):
         st_atime = os.stat(self.path).st_atime
-        st_mtime = datetime.datetime.now().timestamp()
+        st_mtime = time.time()
         os.utime(self.path, (st_atime, st_mtime))
         self._logger.info('Update the note mtime to %s', datetime.datetime.fromtimestamp(st_mtime))
 
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_client.py b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_client.py
index 4a797abe76f..2442942a7ce 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_client.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_client.py
@@ -46,7 +46,7 @@ def setUp(self):
                 for line in fq.readlines():
                     self.sample_records.append(line[20:])
 
-        _TestSender.instances.clear()
+        del _TestSender.instances[:]
 
     def test_telemetry_client_without_flush(self):
         client = CliTelemetryClient(sender=_TestSender)
@@ -76,7 +76,7 @@ def test_telemetry_client_without_flush(self):
         self.assertEqual(1, len(sender.data))
 
         # default batch size is 100, ensure data is sent after accumulation
-        sender.data.clear()
+        del sender.data[:]
         count = 0
         for r in self.sample_records:
             client.add(r, flush=True)
@@ -84,7 +84,7 @@ def test_telemetry_client_without_flush(self):
             count += 1
             if not count % 100:
                 self.assertEqual(1, len(sender.data))
-                sender.data.clear()
+                del sender.data[:]
             else:
                 self.assertEqual(0, len(sender.data))
 
@@ -92,7 +92,7 @@ def test_telemetry_client_without_flush(self):
 class TestNoRetrySender(unittest.TestCase):
     def setUp(self):
         # build some data in the form of envelops
-        _TestSender.instances.clear()
+        del _TestSender.instances[:]
         client = CliTelemetryClient(sender=_TestSender)
         with open(os.path.join(TEST_RESOURCE_FOLDER, 'cache'), mode='r') as fq:
             for line in fq.readlines()[:5]:
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_logging.py b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_logging.py
index add10f77a3e..1b7cbdc017e 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_logging.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_logging.py
@@ -28,7 +28,7 @@ class TestTelemetryUploadLogging(unittest.TestCase):
     def setUp(self):
         self._original_handlers = logging.root.handlers
         self._original_level = logging.root.level
-        logging.root.handlers.clear()
+        del logging.root.handlers[:]
 
     def tearDown(self):
         for handler in logging.root.handlers:
@@ -36,7 +36,7 @@ def tearDown(self):
                 if handler.stream:
                     handler.stream.close()
 
-        logging.root.handlers.clear()
+        del logging.root.handlers[:]
         logging.root.setLevel(self._original_level)
         for handler in self._original_handlers:
             logging.root.addHandler(handler)
@@ -55,7 +55,8 @@ def test_config_logging_for_upload_process(self):
 
         with open(log_file, mode='r') as fq:
             content = fq.read().strip('\n')
-            self.assertTrue(content.endswith(random_name))
+            self.assertTrue(content.endswith(random_name),
+                            'Log content {} does not contain {}'.format(content, random_name))
 
     def test_config_logging_for_upload_process_nonexist(self):
         temp_dir = tempfile.mktemp()
@@ -73,7 +74,8 @@ def test_config_logging_for_upload_process_nonexist(self):
 
         with open(log_file, mode='r') as fq:
             content = fq.read().strip('\n')
-            self.assertTrue(content.endswith(random_name))
+            self.assertTrue(content.endswith(random_name),
+                            'Log content {} does not contain {}'.format(content, random_name))
 
 
 if __name__ == '__main__':
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_note.py b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_note.py
index 2aa9047f6db..7095f93bfc6 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_note.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/tests/test_telemetry_note.py
@@ -9,6 +9,7 @@
 import shutil
 import stat
 import datetime
+import time
 
 import portalocker
 
@@ -53,7 +54,7 @@ def test_create_telemetry_note_file_from_scratch(self):
                     self.assertFalse(True)
 
     def test_open_telemetry_note_file(self):
-        with open(TelemetryNote.get_file_path(self.workDir), mode='x') as fq:
+        with open(TelemetryNote.get_file_path(self.workDir), mode='w') as fq:
             fq.write(self.SAMPLE_TIME_1.strftime('%Y-%m-%dT%H:%M:%S'))
 
         with TelemetryNote(self.workDir) as note:
@@ -75,10 +76,13 @@ def assert_modify_time(self, file_path, gap):
     def set_modify_time(self, file_path, new_time=None):
         if not new_time:
             new_time = (datetime.datetime.now() - datetime.timedelta(minutes=5))
-        os.utime(file_path, (new_time.timestamp(), new_time.timestamp()))
+
+        expected_timestamp = time.mktime(new_time.timetuple()) + new_time.microsecond / 1E6
+        os.utime(file_path, (expected_timestamp, expected_timestamp))
 
         modify_time = datetime.datetime.fromtimestamp(os.stat(file_path).st_mtime)
-        self.assertEqual(new_time, modify_time)
+        actual_timestamp = time.mktime(modify_time.timetuple()) + modify_time.microsecond / 1E6
+        self.assertAlmostEqual(expected_timestamp, actual_timestamp, delta=1E-6)
 
 
 if __name__ == '__main__':
diff --git a/src/azure-cli-telemetry/azure/cli/telemetry/util.py b/src/azure-cli-telemetry/azure/cli/telemetry/util.py
index f0906a59b66..1224287ceae 100644
--- a/src/azure-cli-telemetry/azure/cli/telemetry/util.py
+++ b/src/azure-cli-telemetry/azure/cli/telemetry/util.py
@@ -60,7 +60,8 @@ def save_payload(config_dir, payload):
 def _create_rotate_file_logger(log_dir):
     cache_name = os.path.join(log_dir, 'telemetry', 'cache')
     try:
-        os.makedirs(os.path.dirname(cache_name), exist_ok=True)
+        if not os.path.exists(os.path.dirname(cache_name)):
+            os.makedirs(os.path.dirname(cache_name))
 
         handler = logging.handlers.RotatingFileHandler(cache_name, maxBytes=128 * 1024, backupCount=100)
         handler.setLevel(logging.INFO)
diff --git a/src/azure-cli-telemetry/setup.cfg b/src/azure-cli-telemetry/setup.cfg
index 76e358d55d7..06e48975918 100644
--- a/src/azure-cli-telemetry/setup.cfg
+++ b/src/azure-cli-telemetry/setup.cfg
@@ -1,6 +1,3 @@
 [bdist_wheel]
 universal=1
 azure-namespace-package=azure-cli-nspkg
-
-[aliases]
-test=pytest
diff --git a/src/azure-cli-telemetry/setup.py b/src/azure-cli-telemetry/setup.py
index 266684721ef..f931ca0787c 100755
--- a/src/azure-cli-telemetry/setup.py
+++ b/src/azure-cli-telemetry/setup.py
@@ -58,7 +58,5 @@
         'azure.cli.telemetry',
         'azure.cli.telemetry.components'
     ],
-    cmdclass=cmdclass,
-    setup_requires=["pytest-runner"],
-    tests_require=["pytest", "mock"]
+    cmdclass=cmdclass
 )
diff --git a/src/azure-cli-telemetry/tox.ini b/src/azure-cli-telemetry/tox.ini
new file mode 100644
index 00000000000..396f8a1254f
--- /dev/null
+++ b/src/azure-cli-telemetry/tox.ini
@@ -0,0 +1,8 @@
+[tox]
+envlist = py37,py36,py27
+skip_missing_interpreters = True
+
+[testenv]
+deps = pytest
+       mock
+commands = pytest