Hk improve tests (#360)

* Improve error message

When forgetting to install the protobuf compiler,
an error message is shown which had a missing
space between two words.

* Remove trailing whitespace

* Read file in one go

There is no reason to traverse the file
line by line if all we care about is the
last character.

This also uses unittests subTest to show
the file which fails the test.

* Fix header comment

* Use subTest to show all failing files

When one file is failing, due to the
fact that all tests contain for loops,
the other failures are swallowed because python
stops running the test on the first assertion.
With subTest one can introduce a subcontext which
allows showing all of the failed files as well a
diagnostic as to which file failed.

* Use enumeration index for line count

Python provides mechanisms for that, no
reason to manually track a count.

* Remove unused imports, only glob once

The globs where repeated a whole bunch of times
which is both information duplication as well
as unneccessary work.

* Remove needless conditionals in front of assert

There is no reason to guard the assertions
in the conditional.

* Switch build to python 3

Python2 is end of life in just a few days
and should not be recommended anymore.

* Improve assertion by using regex match.

The error message is quite good when using the
provided regex matcher.

* Only deploy on master

* Remove trailing whitespace

Author: hanneskaeufler

.travis.yml

@@ -38,7 +38,7 @@ install:
         PROTOBUF_URL="https://github.com/google/protobuf/releases/download/v3.2.0/protobuf-cpp-3.2.0.tar.gz"
         if [ ! -f ${DEPS_DIR}/protobuf/install/bin/protoc ] ; then mkdir -p protobuf ; travis_retry wget --no-check-certificate --quiet -O - ${PROTOBUF_URL} | tar --strip-components=1 -xz -C protobuf ; cd protobuf ; ./configure --prefix=${DEPS_DIR}/protobuf/install ; make ; make install ; fi
         export PATH=${DEPS_DIR}/protobuf/install/bin:${PATH}
-        
+
     # Install proto2cpp for doxygen
     - |
         PROTO2CPP_URL="https://github.com/OpenSimulationInterface/proto2cpp.git"
@@ -50,7 +50,7 @@ env:
     - GH_REPO_ORG=`echo $TRAVIS_REPO_SLUG | cut -d "/" -f 1`
     - GH_REPO_NAME=`echo $TRAVIS_REPO_SLUG | cut -d "/" -f 2`
     - GH_REPO_REF="github.com/$GH_REPO_ORG/$GH_REPO_NAME.git"
-    
+
 # Addons necessary for documentation.
 addons:
     apt:
@@ -60,10 +60,11 @@ addons:
         - doxygen-latex
         - doxygen-gui
         - graphviz
+        - python3-setuptools
 
 
 jobs:
-  include:    
+  include:
     # Run the build script and generate documentation.
     - stage: test OSI protobuf
 
@@ -75,8 +76,8 @@ jobs:
           - cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install -D CMAKE_INSTALL_PREFIX:PATH=${TESTINST_DIR} ..
           - cmake --build .
           - cd ..
-          - python setup.py build
-          - python setup.py sdist
+          - python3 setup.py build
+          - python3 setup.py sdist
           - cd build
           - cmake --build . --target install
           - cd ..
@@ -89,15 +90,15 @@ jobs:
           - echo "GENERATE_TREEVIEW = YES" >> Doxyfile
           - doxygen
           - cd ..
-          - python -m unittest discover tests
+          - python3 -m unittest discover tests
           - mv VERSION.SAVED VERSION
           - sh convert-to-proto3.sh
           - mkdir -p build3
           - cd build3
           - cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install ..
           - cmake --build .
           - cd ..
-      
+
     - script:
           - cd "${TRAVIS_BUILD_DIR}"
           - bash convert-to-proto3.sh
@@ -107,8 +108,8 @@ jobs:
           - cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install -D CMAKE_INSTALL_PREFIX:PATH=${TESTINST_DIR} ..
           - cmake --build .
           - cd ..
-          - python setup.py build
-          - python setup.py sdist
+          - python3 setup.py build
+          - python3 setup.py sdist
           - cd build
           - cmake --build . --target install
           - cd ..
@@ -121,7 +122,7 @@ jobs:
           - echo "GENERATE_TREEVIEW = YES" >> Doxyfile
           - doxygen
           - cd ..
-          - python -m unittest discover tests
+          - python3 -m unittest discover tests
           - mv VERSION.SAVED VERSION
           - sh convert-to-proto3.sh
           - mkdir -p build3
@@ -139,8 +140,8 @@ jobs:
           - cmake -D CMAKE_PREFIX_PATH:PATH=${DEPS_DIR}/protobuf/install -D CMAKE_INSTALL_PREFIX:PATH=${TESTINST_DIR} ..
           - cmake --build .
           - cd ..
-          - python setup.py build
-          - python setup.py sdist
+          - python3 setup.py build
+          - python3 setup.py sdist
           - cd build
           - cmake --build . --target install
           - cd ..
@@ -153,7 +154,7 @@ jobs:
           - echo "GENERATE_TREEVIEW = YES" >> Doxyfile
           - doxygen
           - cd ..
-          - python -m unittest discover tests
+          - python3 -m unittest discover tests
           - mv VERSION.SAVED VERSION
           - sh convert-to-proto3.sh
           - mkdir -p build3
@@ -163,11 +164,11 @@ jobs:
           - cd ..
 
     # Deploy the documentation on github (only for master branch).
-      deploy:        
+      deploy:
           provider: pages
           skip_cleanup: true
           local_dir: doc/html
           keep-history: true
           github_token: ${GH_REPO_TOKEN}
-          on: 
-          branch: master 
+          on:
+              branch: master

README.md

@@ -85,7 +85,7 @@ $ sudo apt-get install cmake
 ```
 Install `pip3` and missing python packages:
 ```bash
-$ sudo apt-get install python3-pip python-setuptools
+$ sudo apt-get install python3-pip python3-setuptools
 ```
 Install `protobuf` 3.0.0:
 ```bash
@@ -109,10 +109,10 @@ Local:
 ```bash
 $ git clone https://github.com/OpenSimulationInterface/open-simulation-interface.git
 $ cd open-simulation-interface
-$ sudo pip3 install virtualenv 
+$ sudo pip3 install virtualenv
 $ virtualenv -p python3 venv
 $ source venv/bin/activate
-$ pip install .
+$ python3 -m pip install .
 ```
 
 Global:

setup.py

@@ -1,4 +1,4 @@
-#! python
+#!/usr/local/env python3
 # -*- coding: UTF-8 -*-
 
 import os
@@ -39,8 +39,8 @@ def find_protoc():
         if protoc is None:
             sys.stderr.write(
                 'protoc not found. Is protobuf-compiler installed? \n'
-                'Alternatively, you can point the PROTOC environment variable'
-                'at a local version.')
+                'Alternatively, you can point the PROTOC environment variable '
+                'to a local version.')
             sys.exit(1)
         return protoc
 

tests/test_comment_type.py

@@ -1,26 +1,25 @@
 import sys
 import unicodedata
 import re
-from glob import *
+import glob
 import unittest
 
+PROTO_FILES = glob.glob("*.proto")
 
 class TestCommentType(unittest.TestCase):
-    ''' Test class for mandatory new line. ''' 
+    ''' Test class for mandatory comments. '''
 
     def test_brief_necessity(self):
         ''' Test the necessity of "brief" comment. '''
 
-        for file in glob("*.proto"):
-            with open(file, "rt") as fin:
-                i = 0
+        for file in PROTO_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
                 noMessage = 0
                 noComment = 0
                 hasBrief = False
                 saveStatement = ""
 
-                for line in fin:
-                    i += 1
+                for i, line in enumerate(fin, start=1):
 
                     # Divide statement and comment. Concatenate multi line statements.
 
@@ -57,7 +56,7 @@ def test_brief_necessity(self):
                         noComment += 1;
                         if comment.find("\\brief") != -1:
                             hasBrief = True
-                    
+
                     elif len(saveStatement) == 0:
                         if re.search(r"\bmessage\b", statement) is not None or re.search(r"\bextend\b",statement) is not None:
                             self.assertTrue(hasBrief, file + " in line " + str(i - 1) + ": \\brief section in comment is missing for: '" + statement + "'")
@@ -73,18 +72,15 @@ def test_brief_necessity(self):
     def test_min_two_lines(self):
         ''' Test to check if short comment is of minimum two lines. '''
 
-        for file in glob("*.proto"):
-            with open(file, "rt") as fin:
-                i = 0
+        for file in PROTO_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
                 isEnum = False
                 noMessage = 0
                 noComment = 0
                 hasBrief = False
                 saveStatement = ""
 
-                for line in fin:
-                    i += 1
-
+                for i, line in enumerate(fin, start=1):
                     # Divide statement and comment. Concatenate multi line statements.
 
                     # Search for comment ("//").
@@ -129,19 +125,15 @@ def test_min_two_lines(self):
     def test_comment_existence(self):
         ''' Test to check if every message, extend , statement or enum has a comment. '''
 
-        for file in glob("*.proto"):
-
-            with open(file, "rt") as fin:
-                i = 0
+        for file in PROTO_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
                 isEnum = False
                 noMessage = 0
                 noComment = 0
                 hasBrief = False
                 saveStatement = ""
 
-                for line in fin:
-                    i += 1                    
-
+                for i, line in enumerate(fin, start=1):
                     # Divide statement and comment. Concatenate multi line statements.
 
                     # Search for comment ("//").

tests/test_doxygen_output.py

@@ -1,51 +1,32 @@
-import sys
-import unicodedata
 import re
-from glob import *
+import glob
 import unittest
 
+DOC_FILES = glob.glob("doc/html/*.htm*")
 
 class TestDoxygenOutput(unittest.TestCase):
     """ Test class for the doxygen output. """
 
     def test_hash(self):
         ''' Test case is checking if there are illegal hash chars in the documentation. -> doxygen link not found.  '''
-        for file in glob("doc/html/*.htm*"):
-            with open(file, "rt") as fin:
-                i = 0
-
-                for line in fin:
-                    i += 1                    
-                    matchHash = re.search(r"([\s>]|^)#\w(\S)*", line)
-
-                    if matchHash is not None:
-                        self.assertIsNone(matchHash, file + " in line " + str(i) + ": not permitted hash found. Search for: '"+ line[matchHash.start():matchHash.end()])
+        for file in DOC_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
+                for i, line in enumerate(fin, start=1):
+                    self.assertNotRegex(line, r"([\s>]|^)#\w(\S)*", file + " in line " + str(i) + ": not permitted hash found.")
 
 
     def test_slash_triplet(self):
         ''' Test case is checking if there are slash triplets in the documentation. -> doxygen didn't interpret something properly. '''
 
-        for file in glob("doc/html/*.htm*"):
-            with open(file, "rt") as fin:
-                i = 0
-
-                for line in fin:
-                    i += 1
-                    matchHash = re.search(r"([\s>]|^)///\s*",line)
-
-                    if matchHash is not None:
-                        self.assertIsNone(matchHash, file + " in line " + str(i) + ": not permitted slash triplet found. Search for: '"+line[matchHash.start():matchHash.end()])
+        for file in DOC_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
+                for i, line in enumerate(fin, start=1):
+                    self.assertNotRegex(line, r"([\s>]|^)///\s*", file + " in line " + str(i) + ": not permitted slash triplet found.")
 
 
     def test_backslash_triplet(self):
         ''' Test case is checking if there are backslash triplets in the documentation. -> doxygen didn't interpret something properly. '''
-        for file in glob("doc/html/*.htm*"):
-            with open(file, "rt") as fin:
-                i = 0
-
-                for line in fin:
-                    i += 1
-                    matchHash = re.search(r"([\s>]|^)\\\\\\\s*",line)
-
-                    if matchHash is not None:
-                        self.assertIsNone(matchHash, file + " in line " + str(i) + ": not permitted backslash triplet found. Search for: '"+line[matchHash.start():matchHash.end()])
+        for file in DOC_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
+                for i, line in enumerate(fin, start=1):
+                    self.assertNotRegex(line, r"([\s>]|^)\\\\\\\s*", file + " in line " + str(i) + ": not permitted backslash triplet found.")

tests/test_invalid_comment.py

@@ -1,29 +1,22 @@
-import sys
-import unicodedata
-import re
-from glob import *
+import glob
 import unittest
 
+PROTO_FILES = glob.glob("*.proto")
+
 class TestInvalidCommentType(unittest.TestCase):
     """Test class for invalid comment types"""
 
     def test_triple_slash(self):
         ''' Test to check if more than two forward slash('/') are present in comment section of proto file. '''
-        for file in glob("*.proto"):
-            with open(file, "rt") as fin:
-                i = 0
-
-                for line in fin:
-                    i += 1
+        for file in PROTO_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
+                for i, line in enumerate(fin, start=1):
                     self.assertEqual(line.find("///"), -1, file + " in line " + str(i) + ": not permitted use of '///' ")
 
     def test_comments_invalid_syntax(self):
         ''' Test to check if comments are given using invalid syntax '/*' or '*/' '''
-        for file in glob("*.proto"):
-            with open(file, "rt") as fin:
-                i = 0
-
-                for line in fin:
-                    i += 1
+        for file in PROTO_FILES:
+            with open(file, "rt") as fin, self.subTest(file=file):
+                for i, line in enumerate(fin, start=1):
                     self.assertEqual(line.find("/*"), -1, file + " in line " + str(i) + ": not permitted use of '/*' ")
                     self.assertEqual(line.find("*/"), -1, file + " in line " + str(i) + ": not permitted use of '*/' ")