Merge pull request #117 from PQClean/run-only-diffs

Be more selective in test runs

.circleci/config.yml

@@ -4,22 +4,27 @@ version: 2
   machine: true
   steps:
     - checkout
-    - run:
+    - run: &checkouttask
         name: Pull submodules
         command: |
           git submodule init
           git submodule update
+          git checkout $CIRCLECI_BRANCH
+          git reset --hard $CIRCLECI_SHA1
     - run:
         name: Install the emulation handlers
         command: docker run --rm --privileged multiarch/qemu-user-static:register --reset
     - run:
         name: Run the tests in a container
         command: |
-          docker run -e CI=true --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c "
+          docker run -e CI=true -e PQCLEAN_ONLY_DIFF=1 --rm -v `pwd`:`pwd` -w `pwd` "pqclean/ci-container:$ARCH" /bin/bash -c "
           uname -a &&
           export CC=${CC} &&
           pip3 install -r requirements.txt &&
-          cd test && python3 -m nose --rednose --verbose"
+          mkdir test-results &&
+          cd test && python3 -m nose --rednose --verbose --with-xunit --xunit-file=../test-results/nosetests.xml"
+    - store_test_results:
+        path: test-results
 
 .native_job: &nativejob
   docker:
@@ -27,17 +32,18 @@ version: 2
   steps:
     - checkout
     - run:
-        name: Pull submodules
+        <<: *checkouttask
-        command: |
-          git submodule init
-          git submodule update
     - run:
         name: Run tests
         command: |
           export CC=${CC}
-          pip3 install -r requirements.txt &&
+          export PQCLEAN_ONLY_DIFF=1
-          cd test && python3 -m nose --rednose --verbose
+          pip3 install -r requirements.txt
-
+          mkdir test-results
+          cd test
+          python3 -m nose --rednose --verbose --with-xunit --xunit-file=../test-results/nosetests.xml
+    - store_test_results:
+        path: test-results
 
 jobs:
   arm64-gcc:

.travis.yml

@@ -6,8 +6,15 @@ matrix:
       compiler: clang
       before_install:
         - pip3 install -r requirements.txt
+      before_script:
+        - git config --replace-all remote.origin.fetch +refs/heads/*:refs/remotes/origin/*
+        - git fetch --unshallow
+        - git checkout $TRAVIS_BRANCH
+        - git reset --hard $TRAVIS_COMMIT
       script:
         - "cd test && python3 -m nose --rednose --verbose"
+      env:
+        PQCLEAN_ONLY_DIFF: 1
       addons:
         homebrew:
           packages:
@@ -21,7 +28,13 @@ matrix:
           packages:
             - astyle
             - gcc@8
+      env:
+        PQCLEAN_ONLY_DIFF: 1
       before_install:
+        - git config --replace-all remote.origin.fetch +refs/heads/*:refs/remotes/origin/*
+        - git fetch --unshallow
+        - git checkout $TRAVIS_BRANCH
+        - git reset --hard $TRAVIS_COMMIT
         - pip3 install -r requirements.txt
         - brew link gcc
         - export PATH="/usr/local/bin:$PATH"

appveyor.yml

@@ -5,7 +5,10 @@ image: Visual Studio 2017
 build:
   verbosity: minimal
 
+shallow_clone: false
+
 environment:
+  PQCLEAN_ONLY_DIFF: 1
   matrix:
     - BITS: 64
     - BITS: 32
@@ -19,6 +22,11 @@ init:
   - set PATH="C:\\Python37";"C:\\Python37\Scripts";%PATH%
 
 build_script:
+  - git config --replace-all remote.origin.fetch +refs/heads/*:refs/remotes/origin/*
+  - git fetch --all
+  - git checkout %APPVEYOR_REPO_BRANCH%
+  - git reset --hard %APPVEYOR_REPO_COMMIT%
+  - git diff --name-only origin/master
   - python -m pip install -r requirements.txt
   - cd test
     # Download Astyle to local folder because putting it in PATH doesn't work

test/Makefile

@@ -20,6 +20,9 @@ CFLAGS=-O3 -Wall -Wextra -Wpedantic -Werror -std=c99 \
 	   -fstrict-aliasing -fno-common -pipe \
 	   -I$(COMMON_DIR) $(EXTRAFLAGS)
 
+# Number of tests run for functests
+NTESTS=1
+
 all: $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION) \
 	$(DEST_DIR)/testvectors_$(SCHEME)_$(IMPLEMENTATION) \
 	$(DEST_DIR)/printparams_$(SCHEME)_$(IMPLEMENTATION)
@@ -50,7 +53,7 @@ $(DEST_DIR)/test_common_%: common/%.c $(COMMON_FILES)
 
 $(DEST_DIR)/functest_$(SCHEME)_$(IMPLEMENTATION): build-scheme crypto_$(TYPE)/functest.c $(COMMON_FILES) $(COMMON_DIR)/randombytes.c $(COMMON_HEADERS)
 	mkdir -p $(DEST_DIR)
-	$(CC) $(CFLAGS) -DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) -I$(SCHEME_DIR) crypto_$(TYPE)/functest.c $(COMMON_FILES) $(COMMON_DIR)/notrandombytes.c -o $@ -L$(SCHEME_DIR) -l$(SCHEME)_$(IMPLEMENTATION)
+	$(CC) $(CFLAGS) -DNTESTS=$(NTESTS) -DPQCLEAN_NAMESPACE=PQCLEAN_$(SCHEME_UPPERCASE)_$(IMPLEMENTATION_UPPERCASE) -I$(SCHEME_DIR) crypto_$(TYPE)/functest.c $(COMMON_FILES) $(COMMON_DIR)/notrandombytes.c -o $@ -L$(SCHEME_DIR) -l$(SCHEME)_$(IMPLEMENTATION)
 
 $(DEST_DIR)/testvectors_$(SCHEME)_$(IMPLEMENTATION): build-scheme crypto_$(TYPE)/testvectors.c $(COMMON_FILES) $(COMMON_DIR)/notrandombytes.c $(COMMON_HEADERS)
 	mkdir -p $(DEST_DIR)

test/crypto_kem/functest.c

@@ -5,7 +5,9 @@
 #include "api.h"
 #include "randombytes.h"
 
+#ifndef NTESTS
 #define NTESTS 5
+#endif
 
 const uint8_t canary[8] = {
     0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF

test/crypto_sign/functest.c

@@ -6,7 +6,10 @@
 #include "api.h"
 #include "randombytes.h"
 
+#ifndef NTESTS
 #define NTESTS 5
+#endif
+
 #define MLEN 32
 
 const uint8_t canary[8] = {

test/helpers.py

@@ -7,6 +7,8 @@ import sys
 
 import pqclean
 
+import logging
+
 
 def run_subprocess(command, working_dir='.', env=None, expected_returncode=0):
     """
@@ -113,7 +115,7 @@ def ensure_available(executable):
     raise AssertionError("{} not available on CI".format(executable))
 
 
-def permit_test(testname, thing, **args):
+def permit_test(testname, thing, *args, **kwargs):
     if 'PQCLEAN_ONLY_TESTS' in os.environ:
         if not(testname.lower() in os.environ['PQCLEAN_ONLY_TESTS'].lower().split(',')):
             return False
@@ -142,13 +144,14 @@ def permit_test(testname, thing, **args):
             return False
 
     if 'PQCLEAN_ONLY_DIFF' in os.environ:
-        if shutil.which('git') != None:
+        if shutil.which('git') is not None:
             # if we're on a non-master branch, and the only changes are in schemes,
             # only run tests on those schemes
             branch_result = subprocess.run(
                 ['git', 'status', '--porcelain=2', '--branch'],
                 stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT
+                stderr=subprocess.STDOUT,
+                cwd="..",
             )
             # ensure we're in a working directory
             if branch_result.returncode != 0:
@@ -161,14 +164,18 @@ def permit_test(testname, thing, **args):
                         return True
             # where are there changes?
             diff_result = subprocess.run(
-                ['git', 'diff', '--name-only', 'master'],
+                ['git', 'diff', '--name-only', 'origin/master'],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT
             )
-            assert(diff_result.returncode == 0), "Got unexpected return code {}".format(diff_result.returncode)
+            assert diff_result.returncode == 0, \
+                "Got unexpected return code {}".format(diff_result.returncode)
             for diff_line in diff_result.stdout.decode('utf-8').splitlines():
                 # don't skip test if there are any changes outside schemes
-                if not(diff_line.startswith('crypto_kem')) and not (diff_line.startswith('crypto_sign')):
+                if (not diff_line.startswith('crypto_kem') and
+                        not diff_line.startswith('crypto_sign')):
+                    logging.info("Running all tests as there are changes "
+                                 "outside of schemes")
                     return True
                 # do test if the scheme in question has been changed
                 if diff_line.startswith(thing.path(base='')):
@@ -177,3 +184,15 @@ def permit_test(testname, thing, **args):
             return False
 
     return True
+
+
+def filtered_test(func):
+    funcname = func.__name__[len("check_"):]
+
+    @functools.wraps(func)
+    def wrapper(*args, **kwargs):
+        if permit_test(funcname, *args, **kwargs):
+            return func(*args, **kwargs)
+        else:
+            raise unittest.SkipTest("Test disabled by filter")
+    return wrapper

test/pqclean.py

@@ -55,7 +55,7 @@ class Scheme:
         metafile = os.path.join(self.path(), 'META.yml')
         try:
             with open(metafile, encoding='utf-8') as f:
-                metadata = yaml.load(f.read())
+                metadata = yaml.safe_load(f.read())
                 return metadata
         except Exception as e:
             print("Can't open {}: {}".format(metafile, e))

test/test_api_h.py

@@ -5,14 +5,14 @@ import helpers
 import pqclean
 
 
-def test_preprocessor():
+def test_api_h():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('preprocessor', implementation):
+            yield check_api_h, implementation
-                yield check_preprocessor, implementation
 
 
-def check_preprocessor(implementation: pqclean.Implementation):
+@helpers.filtered_test
+def check_api_h(implementation: pqclean.Implementation):
     apipath = os.path.join(implementation.path(), 'api.h')
     errors = []
     p = re.compile(r'^\s*#include\s*"')
@@ -25,6 +25,7 @@ def check_preprocessor(implementation: pqclean.Implementation):
             "Prohibited external include in api.h" + "".join(errors)
         )
 
+
 if __name__ == "__main__":
     try:
         import nose2

test/test_char.py

@@ -17,7 +17,6 @@ def test_char():
         )
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('char', implementation):
             yield check_char, implementation
 
 
@@ -30,6 +29,7 @@ def walk_tree(ast):
         yield from walk_tree(child)  # recursively yield prohibited nodes
 
 
+@helpers.filtered_test
 @helpers.skip_windows()
 def check_char(implementation):
     errors = []

test/test_common.py

@@ -12,7 +12,8 @@ import helpers
 def test_common():
     for d in os.listdir('common'):
         primitive = re.sub(r"\.c$", "", d)
-        if helpers.permit_test('common', None): yield check_common, primitive
+        if helpers.permit_test('common', None):
+            yield check_common, primitive
 
 
 def check_common(primitive):

test/test_compile_lib.py

@@ -10,10 +10,10 @@ import helpers
 def test_compile_lib():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('compile_lib', implementation):
             yield check_compile_lib, implementation
 
 
+@helpers.filtered_test
 def check_compile_lib(implementation):
     helpers.make('clean', working_dir=implementation.path())
     helpers.make(working_dir=implementation.path())

test/test_duplicate_consistency.py

@@ -8,33 +8,54 @@ import helpers
 import unittest
 import yaml
 
-helpers.skip_windows()
+
+def _skipped_test(*args, **kwargs):
+    raise unittest.SkipTest("Skipped consistency check")
+
+
 def test_duplicate_consistency():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('duplicate_consistency', implementation): 
+            if not helpers.permit_test('duplicate_consistency',
-                if os.path.isfile(os.path.join('duplicate_consistency', '{}_{}.yml'.format(scheme.name, implementation.name))):
+                                       implementation):
-                    metafile = os.path.join('duplicate_consistency', '{}_{}.yml'.format(implementation.scheme.name, implementation.name))
+                yield _skipped_test, implementation
+                continue
+
+            if os.path.isfile(
+                    os.path.join(
+                        'duplicate_consistency',
+                        '{}_{}.yml'.format(scheme.name, implementation.name))):
+                metafile = os.path.join(
+                    'duplicate_consistency',
+                    '{}_{}.yml'.format(scheme.name, implementation.name))
                 with open(metafile, encoding='utf-8') as f:
-                        metadata = yaml.load(f.read())
+                    metadata = yaml.safe_load(f.read())
                     for group in metadata['consistency_checks']:
-                            source = pqclean.Implementation.by_name(group['source']['scheme'], group['source']['implementation'])
+                        source = pqclean.Implementation.by_name(
+                                group['source']['scheme'],
+                                group['source']['implementation'])
                         for file in group['files']:
-                                yield check_duplicate_consistency, implementation, source, file
+                            yield (check_duplicate_consistency, implementation,
+                                   source, file)
+
 
 def file_get_contents(filename):
     with open(filename) as f:
         return f.read()
 
+
+@helpers.skip_windows()
 def check_duplicate_consistency(implementation, source, file):
     transformed_src = helpers.run_subprocess(
-        ['sed', '-e', 's/{}/{}/g'.format(source.namespace_prefix(), implementation.namespace_prefix()), os.path.join(source.path(), file)]
+        ['sed', '-e', 's/{}/{}/g'.format(source.namespace_prefix(),
+         implementation.namespace_prefix()), os.path.join(source.path(), file)]
     )
     this_src = file_get_contents(os.path.join(implementation.path(), file))
     print(os.path.join(implementation.path(), file))
     print(this_src)
     assert(transformed_src == this_src)
 
+
 if __name__ == '__main__':
     try:
         import nose2

test/test_dynamic_memory.py

@@ -4,19 +4,17 @@ Checks that no dynamic memory functions are used
 
 import pqclean
 import helpers
-import sys
-import unittest
 
 
 def test_dynamic_memory():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('dynamic_memory', implementation): 
             # Keep this loop outside, to allow multiple assertions
             for function in ['malloc', 'free', 'realloc', 'calloc']:
                 yield (check_dynamic_memory, implementation, function)
 
 
+@helpers.filtered_test
 @helpers.skip_windows()
 def check_dynamic_memory(implementation, function):
     # 'make' will take care of not rebuilding existing library files

test/test_format.py

@@ -5,15 +5,16 @@ import pqclean
 def test_formatting():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('format', implementation): 
             yield check_format, implementation
 
 
+@helpers.filtered_test
 def check_format(implementation: pqclean.Implementation):
     helpers.ensure_available('astyle')
     cfiles = implementation.cfiles()
     hfiles = implementation.hfiles()
-    result = helpers.run_subprocess(['astyle',
+    result = helpers.run_subprocess(
+        ['astyle',
          '--dry-run',
          '--options=../.astylerc',
          *cfiles,

test/test_functest.py

@@ -14,17 +14,16 @@ import helpers
 def test_functest():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('functest', implementation):
             yield check_functest, implementation
 
 
 def test_functest_sanitizers():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('functest_sanitizers', implementation):
             yield check_functest_sanitizers, implementation
 
 
+@helpers.filtered_test
 def check_functest(implementation):
     helpers.make('functest',
                  TYPE=implementation.scheme.type,
@@ -41,6 +40,7 @@ def check_functest(implementation):
     )
 
 
+@helpers.filtered_test
 @helpers.skip_windows()
 @helpers.slow_test
 def check_functest_sanitizers(implementation):

test/test_license.py

@@ -11,10 +11,10 @@ import helpers
 def test_license():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('license', implementation): 
             yield check_license, implementation
 
 
+@helpers.filtered_test
 def check_license(implementation):
     p1 = os.path.join(implementation.path(), 'LICENSE')
     p2 = os.path.join(implementation.path(), 'LICENSE.txt')

test/test_linter.py

@@ -12,10 +12,10 @@ additional_flags = []
 def test_clang_tidy():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('linter', implementation):
             yield check_tidy, implementation
 
 
+@helpers.filtered_test
 @helpers.skip_windows()
 def check_tidy(implementation: pqclean.Implementation):
     helpers.ensure_available('clang-tidy')

test/test_makefile_dependencies.py

@@ -8,12 +8,22 @@ import pqclean
 import helpers
 import glob
 import datetime
+import unittest
+
+
+def _skipped_test(*args, **kwargs):
+    """Used to indicate skipped tests"""
+    raise unittest.SkipTest("Skipped makefile dependencies test")
 
 
 def test_makefile_dependencies():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('makefile_dependencies', implementation):
+            if not helpers.permit_test(
+                    'makefile_dependencies', implementation):
+                yield _skipped_test, implementation
+                continue
+
             # initial build - want to have *all* files in place at beginning
             helpers.make('clean', working_dir=implementation.path())
             helpers.make(working_dir=implementation.path())

test/test_metadata.py

@@ -10,10 +10,10 @@ import pqclean
 
 def test_metadata():
     for scheme in pqclean.Scheme.all_schemes():
-        if helpers.permit_test('metadata', scheme):
         yield check_metadata, scheme
 
 
+@helpers.filtered_test
 def check_metadata(scheme):
     metadata = scheme.metadata()
 

test/test_metadata_sizes.py

@@ -8,10 +8,10 @@ import helpers
 def test_metadata_sizes():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('metadata_sizes', implementation):
             yield check_metadata_sizes, implementation
 
 
+@helpers.filtered_test
 def check_metadata_sizes(implementation):
     metadata = implementation.scheme.metadata()
     impl_meta = next((impl for impl in metadata['implementations']

test/test_nistkat.py

@@ -14,17 +14,21 @@ import pqclean
 import helpers
 import unittest
 
+
 def test_nistkat():
     for scheme in pqclean.Scheme.all_schemes():
-        if scheme.type != 'kem': continue
+        if scheme.type != 'kem':
+            continue
         for implementation in scheme.implementations:
-            if helpers.permit_test('nistkat', implementation):
             yield check_nistkat, implementation
 
 
+@helpers.filtered_test
 def check_nistkat(implementation):
     if implementation.scheme.name == "kyber768":
-        raise unittest.SkipTest("Temporarily skip NIST KAT check for kyber768 since it's an outdated implementation")
+        raise unittest.SkipTest(
+            "Temporarily skip NIST KAT check for kyber768 since it's "
+            "an outdated implementation")
     helpers.make('nistkat',
                  TYPE=implementation.scheme.type,
                  SCHEME=implementation.scheme.name,

test/test_no_symlinks.py

@@ -10,10 +10,10 @@ import helpers
 def test_no_symlinks():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('no_symlinks', implementation):
             yield check_no_symlinks, implementation
 
 
+@helpers.filtered_test
 def check_no_symlinks(implementation):
     for file in os.listdir(implementation.path()):
         fpath = os.path.join(implementation.path(), file)

test/test_preprocessor.py

@@ -5,10 +5,10 @@ import helpers
 def test_preprocessor():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('preprocessor', implementation):
             yield check_preprocessor, implementation
 
 
+@helpers.filtered_test
 def check_preprocessor(implementation: pqclean.Implementation):
     cfiles = implementation.cfiles()
     hfiles = implementation.hfiles()

test/test_symbol_namespace.py

@@ -12,10 +12,10 @@ import unittest
 def test_symbol_namespace():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('symbol_namespace', implementation):
             yield check_symbol_namespace, implementation
 
 
+@helpers.filtered_test
 def check_symbol_namespace(implementation):
     if sys.platform not in ['linux', 'darwin']:
         raise unittest.SkipTest("Unsupported platform")

test/test_testvectors.py

@@ -10,13 +10,8 @@ import helpers
 
 
 def test_testvectors():
-    for scheme in pqclean.Scheme.all_schemes():
+    @helpers.filtered_test
-        for implementation in scheme.implementations:
+    def check_testvectors(implementation):
-            if helpers.permit_test('testvectors', implementation):
-                yield check_vectors, implementation
-
-
-def check_vectors(implementation):
         helpers.make('testvectors',
                      TYPE=implementation.scheme.type,
                      SCHEME=implementation.scheme.name,
@@ -33,6 +28,10 @@ def check_vectors(implementation):
         assert(implementation.scheme.metadata()['testvectors-sha256'].lower()
                == hashlib.sha256(out.encode('utf-8')).hexdigest().lower())
 
+    for scheme in pqclean.Scheme.all_schemes():
+        for implementation in scheme.implementations:
+            yield check_testvectors, implementation
+
 
 if __name__ == '__main__':
     try:

test/test_valgrind.py

@@ -13,11 +13,11 @@ import helpers
 def test_functest():
     for scheme in pqclean.Scheme.all_schemes():
         for implementation in scheme.implementations:
-            if helpers.permit_test('valgrind', implementation):
             yield check_valgrind, implementation
 
 
 @helpers.slow_test
+@helpers.filtered_test
 def check_valgrind(implementation: pqclean.Implementation):
     if (platform.machine() not in ('i386', 'x86_64') or
             platform.system() != 'Linux'):