From dedf47f4369f76394385d7538ad673b3700d5ca1 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Wed, 2 Dec 2020 18:12:17 -0800 Subject: [PATCH 1/2] Refactor temporary test directory creation Now the includeorder, includeguard, and config tests use a temporary directory with their own .styleguide files instead of using the wpiformat .styleguide file. This allowed massively simplifying wpiformat's .styleguide file. --- .styleguide | 28 +- .styleguide-license | 3 +- wpiformat/wpiformat/test/test_cidentlist.py | 17 +- wpiformat/wpiformat/test/test_config.py | 58 ++- wpiformat/wpiformat/test/test_includeguard.py | 331 +++++++++--------- wpiformat/wpiformat/test/test_includeorder.py | 17 +- .../wpiformat/test/test_licenseupdate.py | 47 +-- wpiformat/wpiformat/test/test_tempdir.py | 15 + 8 files changed, 287 insertions(+), 229 deletions(-) create mode 100644 wpiformat/wpiformat/test/test_tempdir.py diff --git a/.styleguide b/.styleguide index 6ffecfdd..d0f7a4da 100644 --- a/.styleguide +++ b/.styleguide @@ -1,33 +1,7 @@ -cppHeaderFileInclude { - \.h$ - \.hpp$ - \.inc$ -} - -cppSrcFileInclude { - \.cpp$ -} - -# Extra "/" used by unit tests generatedFileExclude { - /cpplint\.py$ + cpplint\.py$ } modifiableFileExclude { \.png$ } - -licenseUpdateExclude { - Excluded\.h$ -} - -# Ordered this way to ensure tests find longest match -includeGuardRoots { - wpiformat/ - wpiformat/Test/ -} - -# Used by unit tests -includeProject { - ^ctre/ -} diff --git a/.styleguide-license b/.styleguide-license index a3fccde8..77684828 100644 --- a/.styleguide-license +++ b/.styleguide-license @@ -1,2 +1 @@ -/*{padding}Company Name{padding}*/ -/* Copyright (c) {year} Company Name. All Rights Reserved.{padding}*/ +// Copyright (c) {year} FIRST diff --git a/wpiformat/wpiformat/test/test_cidentlist.py b/wpiformat/wpiformat/test/test_cidentlist.py index 39fa7755..79327f97 100644 --- a/wpiformat/wpiformat/test/test_cidentlist.py +++ b/wpiformat/wpiformat/test/test_cidentlist.py @@ -1,6 +1,7 @@ import os from .test_tasktest import * +from .test_tempdir import * from wpiformat.cidentlist import CIdentList @@ -494,4 +495,18 @@ def test_cidentlist(): True, ) - test.run(OutputType.FILE) + with OpenTemporaryDirectory(): + with open(".styleguide", "w") as file: + file.write( + r"""cppHeaderFileInclude { + \.h$ + \.hpp$ + \.inc$ +} + +includeProject { + ^ctre/ +} +""" + ) + test.run(OutputType.FILE) diff --git a/wpiformat/wpiformat/test/test_config.py b/wpiformat/wpiformat/test/test_config.py index 922c9613..56af5297 100644 --- a/wpiformat/wpiformat/test/test_config.py +++ b/wpiformat/wpiformat/test/test_config.py @@ -1,23 +1,45 @@ -import os - +from .tempdir import * from wpiformat.config import Config def test_config(): - config_file = Config(os.path.abspath(os.getcwd()), ".styleguide") - assert config_file.is_modifiable_file( - "." + os.sep + "wpiformat" + os.sep + "javaguidelink.png" - ) - assert config_file.is_generated_file( - "." + os.sep + "wpiformat" + os.sep + "wpiformat" + os.sep + "cpplint.py" - ) + with OpenTemporaryDirectory(): + with open(".styleguide", "w") as file: + file.write( + r"""cppHeaderFileInclude { + \.h$ + \.inc$ +} + +cppSrcFileInclude { + \.cpp$ +} + +# Extra "/" used by unit tests +generatedFileExclude { + /cpplint\.py$ +} + +modifiableFileExclude { + \.png$ +} +""" + ) + + config_file = Config(os.path.abspath(os.getcwd()), ".styleguide") + assert config_file.is_modifiable_file( + "." + os.sep + "wpiformat" + os.sep + "javaguidelink.png" + ) + assert config_file.is_generated_file( + "." + os.sep + "wpiformat" + os.sep + "wpiformat" + os.sep + "cpplint.py" + ) - assert not config_file.is_generated_file( - "." + os.sep + "wpiformat" + os.sep + "diff_cpplint.py" - ) - assert not config_file.is_generated_file( - "." + os.sep + "wpiformat" + os.sep + "update_cpplint.py" - ) - assert not config_file.is_modifiable_file( - "." + os.sep + "wpiformat" + os.sep + "license.txt" - ) + assert not config_file.is_generated_file( + "." + os.sep + "wpiformat" + os.sep + "diff_cpplint.py" + ) + assert not config_file.is_generated_file( + "." + os.sep + "wpiformat" + os.sep + "update_cpplint.py" + ) + assert not config_file.is_modifiable_file( + "." + os.sep + "wpiformat" + os.sep + "license.txt" + ) diff --git a/wpiformat/wpiformat/test/test_includeguard.py b/wpiformat/wpiformat/test/test_includeguard.py index b8a7c0fb..73a6531e 100644 --- a/wpiformat/wpiformat/test/test_includeguard.py +++ b/wpiformat/wpiformat/test/test_includeguard.py @@ -1,6 +1,7 @@ import os from .test_tasktest import * +from .test_tempdir import * from wpiformat.includeguard import IncludeGuard from wpiformat.task import Task @@ -8,171 +9,181 @@ def test_includeguard(): test = TaskTest(IncludeGuard()) - repo_root = os.path.basename(Task.get_repo_root()).upper() + with OpenTemporaryDirectory(): + with open(".styleguide", "w") as file: + # Roots are ordered this way to ensure tests find longest match + file.write( + r"""includeGuardRoots { + wpiformat/ + wpiformat/Test/ +} +""" + ) + repo_root = os.path.basename(Task.get_repo_root()).upper() - # Fix incorrect include guard - test.add_input( - "./Test.h", - "#ifndef WRONG_H" - + os.linesep - + "#define WRONG_C" - + os.linesep - + os.linesep - + "#endif" - + os.linesep, - ) - test.add_output( - "#ifndef " - + repo_root - + "_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_TEST_H_" - + os.linesep, - True, - ) + # Fix incorrect include guard + test.add_input( + "./Test.h", + "#ifndef WRONG_H" + + os.linesep + + "#define WRONG_C" + + os.linesep + + os.linesep + + "#endif" + + os.linesep, + ) + test.add_output( + "#ifndef " + + repo_root + + "_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_TEST_H_" + + os.linesep, + True, + ) - # Ensure nested preprocessor statements are handled properly for incorrect - # include guard - test.add_input( - "./Test.h", - "#ifndef WRONG_H" - + os.linesep - + "#define WRONG_C" - + os.linesep - + os.linesep - + "#if SOMETHING" - + os.linesep - + "// do something" - + os.linesep - + "#endif" - + os.linesep - + "#endif" - + os.linesep, - ) - test.add_output( - "#ifndef " - + repo_root - + "_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_TEST_H_" - + os.linesep - + os.linesep - + "#if SOMETHING" - + os.linesep - + "// do something" - + os.linesep - + "#endif" - + os.linesep - + "#endif // " - + repo_root - + "_TEST_H_" - + os.linesep, - True, - ) + # Ensure nested preprocessor statements are handled properly for incorrect + # include guard + test.add_input( + "./Test.h", + "#ifndef WRONG_H" + + os.linesep + + "#define WRONG_C" + + os.linesep + + os.linesep + + "#if SOMETHING" + + os.linesep + + "// do something" + + os.linesep + + "#endif" + + os.linesep + + "#endif" + + os.linesep, + ) + test.add_output( + "#ifndef " + + repo_root + + "_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_TEST_H_" + + os.linesep + + os.linesep + + "#if SOMETHING" + + os.linesep + + "// do something" + + os.linesep + + "#endif" + + os.linesep + + "#endif // " + + repo_root + + "_TEST_H_" + + os.linesep, + True, + ) - # Don't touch correct include guard - test.add_input( - "./Test.h", - "#ifndef " - + repo_root - + "_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_TEST_H_" - + os.linesep, - ) - test.add_latest_input_as_output(True) + # Don't touch correct include guard + test.add_input( + "./Test.h", + "#ifndef " + + repo_root + + "_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_TEST_H_" + + os.linesep, + ) + test.add_latest_input_as_output(True) - # Fail on missing include guard - test.add_input("./Test.h", "// Empty file" + os.linesep) - test.add_latest_input_as_output(False) + # Fail on missing include guard + test.add_input("./Test.h", "// Empty file" + os.linesep) + test.add_latest_input_as_output(False) - # Verify pragma once counts as include guard - test.add_input("./Test.h", "#pragma once" + os.linesep) - test.add_latest_input_as_output(True) + # Verify pragma once counts as include guard + test.add_input("./Test.h", "#pragma once" + os.linesep) + test.add_latest_input_as_output(True) - # Ensure include guard roots are processed correctly - test.add_input( - "./Test.h", - "#ifndef " - + repo_root - + "_WPIFORMAT_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_WPIFORMAT_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_WPIFORMAT_TEST_H_" - + os.linesep, - ) - test.add_output( - "#ifndef " - + repo_root - + "_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_TEST_H_" - + os.linesep, - True, - ) + # Ensure include guard roots are processed correctly + test.add_input( + "./Test.h", + "#ifndef " + + repo_root + + "_WPIFORMAT_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_WPIFORMAT_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_WPIFORMAT_TEST_H_" + + os.linesep, + ) + test.add_output( + "#ifndef " + + repo_root + + "_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_TEST_H_" + + os.linesep, + True, + ) - # Ensure leading underscores are removed (this occurs if the user doesn't - # include a trailing "/" in the include guard root) - test.add_input( - "./Test/Test.h", - "#ifndef " - + repo_root - + "_WPIFORMAT_TEST_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_WPIFORMAT_TEST_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_WPIFORMAT_TEST_TEST_H_" - + os.linesep, - ) - test.add_output( - "#ifndef " - + repo_root - + "_TEST_H_" - + os.linesep - + "#define " - + repo_root - + "_TEST_H_" - + os.linesep - + os.linesep - + "#endif // " - + repo_root - + "_TEST_H_" - + os.linesep, - True, - ) + # Ensure leading underscores are removed (this occurs if the user doesn't + # include a trailing "/" in the include guard root) + test.add_input( + "./Test/Test.h", + "#ifndef " + + repo_root + + "_WPIFORMAT_TEST_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_WPIFORMAT_TEST_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_WPIFORMAT_TEST_TEST_H_" + + os.linesep, + ) + test.add_output( + "#ifndef " + + repo_root + + "_TEST_H_" + + os.linesep + + "#define " + + repo_root + + "_TEST_H_" + + os.linesep + + os.linesep + + "#endif // " + + repo_root + + "_TEST_H_" + + os.linesep, + True, + ) - test.run(OutputType.FILE) + test.run(OutputType.FILE) diff --git a/wpiformat/wpiformat/test/test_includeorder.py b/wpiformat/wpiformat/test/test_includeorder.py index 19290bed..d8730361 100644 --- a/wpiformat/wpiformat/test/test_includeorder.py +++ b/wpiformat/wpiformat/test/test_includeorder.py @@ -1,6 +1,7 @@ import os from .test_tasktest import * +from .test_tempdir import * from wpiformat.includeorder import IncludeOrder @@ -949,4 +950,18 @@ def test_includeorder(): ) test.add_latest_input_as_output(True) - test.run(OutputType.FILE) + with OpenTemporaryDirectory(): + with open(".styleguide", "w") as file: + file.write( + r"""cppHeaderFileInclude { + \.h$ + \.hpp$ + \.inc$ +} + +includeProject { + ^ctre/ +} +""" + ) + test.run(OutputType.FILE) diff --git a/wpiformat/wpiformat/test/test_licenseupdate.py b/wpiformat/wpiformat/test/test_licenseupdate.py index 85846aed..04faee48 100644 --- a/wpiformat/wpiformat/test/test_licenseupdate.py +++ b/wpiformat/wpiformat/test/test_licenseupdate.py @@ -3,26 +3,13 @@ from pathlib import Path import shutil import subprocess -import tempfile from .test_tasktest import * +from .test_tempdir import * from wpiformat.config import Config from wpiformat.licenseupdate import LicenseUpdate -class OpenTemporaryDirectory: - def __init__(self): - self.prev_dir = os.getcwd() - - def __enter__(self): - self.temp_dir = tempfile.TemporaryDirectory() - os.chdir(self.temp_dir.name) - return self.temp_dir - - def __exit__(self, type, value, traceback): - os.chdir(self.prev_dir) - - def test_licenseupdate(): year = str(date.today().year) @@ -306,10 +293,6 @@ def test_licenseupdate(): True, ) - # Ensure excluded files won't be processed - config_file = Config(os.path.abspath(os.getcwd()), ".styleguide") - assert not task.should_process_file(config_file, "./Excluded.h") - # Create git repo to test license years for commits with OpenTemporaryDirectory(): subprocess.run(["git", "init", "-q"]) @@ -318,11 +301,28 @@ def test_licenseupdate(): with open(".styleguide-license", "w") as file: file.write("// Copyright (c) {year}") with open(".styleguide", "w") as file: - file.write("cppSrcFileInclude {\n" + r"\.cpp$") + file.write( + r"""cppHeaderFileInclude { + \.h$ +} + +cppSrcFileInclude { + \.cpp$ +} + +licenseUpdateExclude { + Excluded\.h$ +} +""" + ) subprocess.run(["git", "add", ".styleguide-license"]) subprocess.run(["git", "add", ".styleguide"]) subprocess.run(["git", "commit", "-q", "-m", '"Initial commit"']) + # Ensure excluded files won't be processed + config_file = Config(os.path.abspath(os.getcwd()), ".styleguide") + assert not task.should_process_file(config_file, "./Excluded.h") + # Add file with commit date of last year and range through this year with open("last-year.cpp", "w") as file: file.write(f"// Copyright (c) 2017-{year}") @@ -392,4 +392,11 @@ def test_licenseupdate(): output, success = task.run_pipeline(config_file, "no-year.cpp", lines) assert output == f"// Copyright (c) {year}\n\n" - test.run(OutputType.FILE) + with open(".styleguide-license", "w") as file: + file.write( + r"""/*{padding}Company Name{padding}*/ +/* Copyright (c) {year} Company Name. All Rights Reserved.{padding}*/ +""" + ) + + test.run(OutputType.FILE) diff --git a/wpiformat/wpiformat/test/test_tempdir.py b/wpiformat/wpiformat/test/test_tempdir.py new file mode 100644 index 00000000..c626bb47 --- /dev/null +++ b/wpiformat/wpiformat/test/test_tempdir.py @@ -0,0 +1,15 @@ +import os +import tempfile + + +class OpenTemporaryDirectory: + def __init__(self): + self.prev_dir = os.getcwd() + + def __enter__(self): + self.temp_dir = tempfile.TemporaryDirectory() + os.chdir(self.temp_dir.name) + return self.temp_dir + + def __exit__(self, type, value, traceback): + os.chdir(self.prev_dir) From 57916e11eee7f48d83a4620ca0de7116020cf44f Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Sat, 26 Dec 2020 19:05:21 -0800 Subject: [PATCH 2/2] Add temp test directory --- wpiformat/wpiformat/test/test_tasktest.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/wpiformat/wpiformat/test/test_tasktest.py b/wpiformat/wpiformat/test/test_tasktest.py index 86c0c5ab..f0914ba2 100644 --- a/wpiformat/wpiformat/test/test_tasktest.py +++ b/wpiformat/wpiformat/test/test_tasktest.py @@ -5,6 +5,7 @@ import os import sys +from .tempdir import * from wpiformat.config import Config @@ -68,6 +69,10 @@ def run(self, output_type): """ assert len(self.inputs) == len(self.outputs) + # Create git repo to test each task + with OpenTemporaryDirectory(): + subprocess.run(["git", "init", "-q"]) + config_file = Config(os.path.abspath(os.getcwd()), ".styleguide") for i in range(len(self.inputs)):