From 9faa1d90c4b2eac2f19ca14f78354405ca9a04dc Mon Sep 17 00:00:00 2001 From: Johannes Lorenz <1042576+JohannesLorenz@users.noreply.github.com> Date: Sat, 13 Aug 2022 21:29:15 +0200 Subject: [PATCH] Fix "check-strings" verification (#6485) PR #6438 does 2 things: 1. Add check-namespace 2. Fix verify script This PR contains only part 2 (and does some preparations for part 1). The goal of the PR is to make CI succeed on master. --- .github/workflows/checks.yml | 6 +- tests/check-strings/README.md | 15 -- tests/check-strings/expectation.txt | 81 --------- tests/check-strings/verify | 60 ------- tests/scripted/README.md | 24 +++ .../{check-strings => scripted}/check-strings | 0 tests/scripted/verify | 162 ++++++++++++++++++ 7 files changed, 189 insertions(+), 159 deletions(-) delete mode 100644 tests/check-strings/README.md delete mode 100644 tests/check-strings/expectation.txt delete mode 100755 tests/check-strings/verify create mode 100644 tests/scripted/README.md rename tests/{check-strings => scripted}/check-strings (100%) create mode 100755 tests/scripted/verify diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 89565ccff..05d078205 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -13,10 +13,10 @@ jobs: run: sudo apt-get install -y python3-tinycss2 - name: Update submodules run: git submodule update --init --recursive - - name: Verify check-strings script - run: tests/check-strings/verify + - name: Verify scripted tests + run: tests/scripted/verify - name: Run check-strings - run: tests/check-strings/check-strings + run: tests/scripted/check-strings shellcheck: runs-on: ubuntu-latest container: koalaman/shellcheck-alpine:v0.4.6 diff --git a/tests/check-strings/README.md b/tests/check-strings/README.md deleted file mode 100644 index 94828b770..000000000 --- a/tests/check-strings/README.md +++ /dev/null @@ -1,15 +0,0 @@ -# Check strings - -This tool checks for invalid strings not necessarily in the code. - -Background: When you move a lot of classes and files, the code may be OK, but -many strings outside the code (translations, style.css, .gitmodules etc.) may -still need to be updated. This script checks whether these strings are still -matching the code. - -Content: - -* `check-strings`: Script to check for invalid strings -* `verify`: Script verifying `check-strings` -* expectation.txt: Expectation file for verify script - diff --git a/tests/check-strings/expectation.txt b/tests/check-strings/expectation.txt deleted file mode 100644 index 53b9ae65e..000000000 --- a/tests/check-strings/expectation.txt +++ /dev/null @@ -1,81 +0,0 @@ - -# .gitmodules - -Error: .gitmodules: Directory does not exist: plugins/sid/resid - -# locale - -Error: data/locale: Source file does not exist: ../../src/gui/BBClipView.cpp -Error: data/locale: Source file does not exist: ../../src/gui/editors/BBEditor.cpp -Error: data/locale: Source file does not exist: ../../src/tracks/BBTrack.cpp -Error: data/locale: Source file does not exist: plugins/SpectrumAnalyzer/SpectrumAnalyzer.cpp -Error: data/locale: Source file does not exist: plugins/SpectrumAnalyzer/SpectrumAnalyzerControlDialog.cpp -Error: data/locale: Source file does not exist: plugins/SpectrumAnalyzer/SpectrumAnalyzerControls.cpp -Error: data/locale: Source file does not exist: plugins/flp_import/FlpImport.cpp -Error: data/locale: Source file does not exist: plugins/opl2/opl2instrument.cpp -Error: data/locale: Source file does not exist: plugins/papu/papu_instrument.cpp -Error: data/locale: Source file does not exist: plugins/sid/sid_instrument.cpp -Error: data/locale: Source file does not exist: src/gui/editors/BBEditor.cpp -Error: data/locale: Source file does not exist: src/gui/widgets/VisualizationWidget.cpp -Error: data/locale: Source file does not exist: src/tracks/BBTrack.cpp -Error: data/locale: Class does not exist in source code: BBClipView -Error: data/locale: Class does not exist in source code: BBEditor -Error: data/locale: Class does not exist in source code: BBTrack -Error: data/locale: Class does not exist in source code: SpectrumAnalyzerControlDialog -Error: data/locale: Class does not exist in source code: SpectrumAnalyzerControls -Error: data/locale: Class does not exist in source code: VisualizationWidget -Error: data/locale: Class does not exist in source code: mixerLineLcdSpinBox -Error: data/locale: Class does not exist in source code: opl2instrument -Error: data/locale: Class does not exist in source code: opl2instrumentView -Error: data/locale: Class does not exist in source code: papuInstrument -Error: data/locale: Class does not exist in source code: papuInstrumentView -Error: data/locale: Class does not exist in source code: pluginBrowser -Error: data/locale: Class does not exist in source code: sidInstrument -Error: data/locale: Class does not exist in source code: sidInstrumentView - -# themes - -Error: data/themes/classic/style.css: Class does not exist in source code: PluginDescList -Error: data/themes/classic/style.css: Class does not exist in source code: effectLabel -Error: data/themes/classic/style.css: Class does not exist in source code: nameLabel -Error: data/themes/classic/style.css: Class does not exist in source code: opl2instrumentView -Error: data/themes/classic/style.css: Class does not exist in source code: sidInstrumentView -Error: data/themes/default/style.css: Class does not exist in source code: PluginDescList -Error: data/themes/default/style.css: Class does not exist in source code: effectLabel -Error: data/themes/default/style.css: Class does not exist in source code: nameLabel -Error: data/themes/default/style.css: Class does not exist in source code: opl2instrumentView -Error: data/themes/default/style.css: Class does not exist in source code: sidInstrumentView - -# patches (checks only plugins/) - -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/metadata.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/modules_comp.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/modules_limit.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/modules_mod.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/organ.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/preset.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/calf/primitives.h -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/calf/src/metadata.cpp -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/swh/flanger_1191.c -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/swh/gsm/short_term.c -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/swh/multivoice_chorus_1201.c -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/swh/retro_flange_1208.c -Error: debian/patches/clang.patch: Source file does not exist: plugins/LadspaEffect/swh/vynil_1905.c -Error: debian/patches/clang.patch: Source file does not exist: plugins/delay/stereodelay.cpp -Error: debian/patches/clang.patch: Source file does not exist: plugins/opl2/fmopl.c - -# debian docs (only one string) - - -# debian/copyright - -Error: debian/copyright: Glob/Path does not exist: data/projects/CoolSongs/Saber-* -Error: debian/copyright: Glob/Path does not exist: plugins/LadspaEffect/calf/src/calf/vumeter.h -Error: debian/copyright: Glob/Path does not exist: plugins/LadspaEffect/swh/gsm/* -Error: debian/copyright: Glob/Path does not exist: plugins/LadspaEffect/swh/util/pitchscale.c -Error: debian/copyright: Glob/Path does not exist: plugins/LadspaEffect/swh/vocoder_1337.c -Error: debian/copyright: Glob/Path does not exist: plugins/opl2/fmopl.* - -# summary - -59 errors. diff --git a/tests/check-strings/verify b/tests/check-strings/verify deleted file mode 100755 index bffcc1a59..000000000 --- a/tests/check-strings/verify +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/python3 - -import subprocess -from pathlib import Path -import tempfile -import os - -lmms_main_path = Path(__file__).resolve().parent.parent.parent - -res = subprocess.run(['git', 'rev-parse', 'HEAD'], capture_output=True, text=True, check=True) -head = res.stdout.rstrip() - -# test with a stable LMMS ref to always get the same expectations -ref = 'f56fc68b669b59f525ad133a0376feaba2a1a4ec' - -# find out a remote URL where we can fetch "ref" from -res = subprocess.run(['git', 'rev-parse', '-q', '--verify', ref+'^{commit}'], capture_output=True, text=True) -if res.returncode == 0: - remote = str(lmms_main_path) -else: - res = subprocess.run(['git', 'remote', 'get-url', 'origin'], capture_output=True, text=True) - if res.returncode == 0: - remote = res.stdout.rstrip() - else: - res = subprocess.run(['git', 'remote', 'get-url', 'upstream'], capture_output=True, text=True) - if res.returncode == 0: - remote = res.stdout.rstrip() - else: - raise RuntimeError( - "Sorry, I can find ref " + ref + " neither in the repo nor in the remotes \"origin\" and \"upstream\"") - -with tempfile.TemporaryDirectory() as tmpdir: - os.chdir(tmpdir) - # get the repo and its submodules - subprocess.run(['git', 'init'], check=True) - subprocess.run(['git', 'remote', 'add', 'origin', remote], check=True) - subprocess.run(['git', 'fetch', '--depth=1', 'origin', ref, head], check=True) - subprocess.run(['git', 'checkout', ref], check=True) - subprocess.run(['git', 'submodule', 'update', '--depth=1', '--init'], check=True) - # checkout the CURRENT check-strings, because it is subject-under-test - subprocess.run(['git', 'checkout', head, 'tests/check-strings/check-strings'], check=True) - # run script - res = subprocess.run([str(lmms_main_path / 'tests' / 'check-strings' / 'check-strings')], - capture_output=True, text=True) - print('--->8--- Script output BEGIN --->8---') - print(res.stdout) - print('--->8--- Script output END --->8---') - if res.stderr: - print('--->8--- Script error output BEGIN --->8---') - print(res.stderr) - print('--->8--- Script error output END --->8---') - # make sure script returned "error" (because we test for errors) and that the output is as expected - if res.returncode == 0: - raise RuntimeError("Script \"check-strings\" no errors, but errors were expected") - else: - if res.stdout != open(str(lmms_main_path / 'tests' / 'check-strings' / 'expectation.txt')).read(): - raise RuntimeError("Script \"check-strings\" returned with different output than in expectation.txt") - -# if we made it until here without an exception, all tests have been passed -print("SUCCESS") diff --git a/tests/scripted/README.md b/tests/scripted/README.md new file mode 100644 index 000000000..7d0c11162 --- /dev/null +++ b/tests/scripted/README.md @@ -0,0 +1,24 @@ +# Scripted checks + +The checks in this directory are not C++ unit tests, but simple scripts that +check the code for issues. + +## Verify + +[verify](verify) is a verification script for the scripts in this folder. + +## Check strings + +[check-strings](check-strings) checks for invalid strings not necessarily in +the code. + +Background: When you move a lot of classes and files, the code may be OK, but +many strings outside the code (translations, style.css, .gitmodules etc.) may +still need to be updated. This script checks whether these strings are still +matching the code. + +## Check namespace + +[check-namespace](check-namespace) checks namespaces and a few related things +like `#ifdef`s. + diff --git a/tests/check-strings/check-strings b/tests/scripted/check-strings similarity index 100% rename from tests/check-strings/check-strings rename to tests/scripted/check-strings diff --git a/tests/scripted/verify b/tests/scripted/verify new file mode 100755 index 000000000..5c27ef898 --- /dev/null +++ b/tests/scripted/verify @@ -0,0 +1,162 @@ +#!/usr/bin/python3 + +import subprocess +from pathlib import Path +import tempfile +import os + + +def set_git_identity(): + """Set local git identity, but only in local repo (to not overwrite user settings)""" + subprocess.run(['git', 'config', 'user.name', 'James Bond']) + subprocess.run(['git', 'config', 'user.email', '007@sis.gov.uk']) + + +def create_file(filename: str, file_content: str): + """Create a file in the current directory and adds it to git""" + Path(filename).parent.mkdir(parents=True, exist_ok=True) + with open(filename, "w") as textfile: + print(file_content, file=textfile) + subprocess.run(['git', 'add', filename], check=True) + + +class ScriptTest(): + def __init__(self, scriptpath: Path): + self.scriptpath = Path(scriptpath) + + def __enter__(self): + """Create temporary, minimal test environment, and change to it""" + self.lmms_tmpdir = tempfile.TemporaryDirectory(dir='.') + os.chdir(self.lmms_tmpdir.name) + # prerequirements + Path('data/themes').mkdir(parents=True) + subprocess.run(['git', 'init', '-b', 'main'], check=True) + set_git_identity() + subprocess.run(['git', 'submodule', 'add', '../../carla', 'plugins/CarlaBase/carla'], check=True) + create_file('src/core/classes.cpp', 'namespace lmms {\nclass TestClass\n}') + create_file('debian/lmms-common.docs', '') + create_file('debian/copyright', '') + create_file('data/locale/de.ts', + '\n' + ' \n' + ' TestClass\n' + ' \n' + ' \n' + ' About LMMS\n' + ' Über LMMS\n' + ' \n' + '\n' + '\n') + subprocess.run(['git', 'commit', '-m', 'Initial commit'], check=True) + return self + + def __exit__(self, type, value, traceback): + """Leave and destroy temporary test environment""" + os.chdir('..') + self.lmms_tmpdir.cleanup() + + def expect(self, expectation: str): + """Check if "expectation" is in the output""" + if expectation not in self.result.stdout: + raise RuntimeError(f'Expected "{expectation}" in script output') + + def run(self, expected_returncode: int = 1): # default: something goes wrong ("to the safe side") + """Run the script, check the exit code and store the result""" + self.result = subprocess.run([str(self.scriptpath)], capture_output=True, text=True) + print('--->8--- Script output BEGIN --->8---') + print(self.result.stdout) + print('--->8--- Script output END --->8---') + if self.result.stderr: + print('--->8--- Script error output BEGIN --->8---') + print(self.result.stderr) + print('--->8--- Script error output END --->8---') + # make sure script returned "error" (because we test for errors) and that the output is as expected + if self.result.returncode != expected_returncode: + raise RuntimeError(f"Script \"check-strings\" returned {self.result.returncode}, " + f"but {expected_returncode} expected") + + +lmms_main_path = Path(__file__).resolve().parent.parent.parent + +with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + check_strings = lmms_main_path / 'tests' / 'scripted' / 'check-strings' + + # create dummy carla repo + Path('carla').mkdir() + os.chdir('carla') + subprocess.run(['git', 'init', '-b', 'main'], check=True) + set_git_identity() + create_file('README.md', 'hello world') + subprocess.run(['git', 'commit', '-m', 'Initial commit'], check=True) + os.chdir('..') + + Path('lmms').mkdir() + os.chdir('lmms') + + # minimal working example + with ScriptTest(check_strings) as test: + test.run(0) # exitcode 0 - no errors expected + test.expect('0 errors') + + with ScriptTest(check_strings) as test: + create_file('data/locale/fr.ts', + '\n' + ' \n' + ' TestClass\n' + ' \n' + ' \n' + ' About LMMS\n' + ' À propos de LMMS\n' + ' \n' + '\n' + '\n') + test.run() + test.expect('Error: data/locale: Source file does not exist: ../../src/core/non-existent.cpp') + test.expect('1 errors') + + with ScriptTest(check_strings) as test: + create_file('data/locale/fr.ts', + '\n' + ' \n' + ' NonExistentClass\n' + ' \n' + ' \n' + ' About LMMS\n' + ' À propos de LMMS\n' + ' \n' + '\n' + '\n') + test.run() + test.expect('Error: data/locale: Class does not exist in source code: NonExistentClass') + test.expect('1 errors') + + with ScriptTest(check_strings) as test: + create_file('data/themes/classic/style.css', + 'lmms--gui--NonExistentClass {' + '\tcolor: #d1d8e4;\n' + '}') + test.run() + test.expect('Error: data/themes/classic/style.css: Class does not exist in source code: NonExistentClass') + test.expect('1 errors') + + with ScriptTest(check_strings) as test: + create_file('debian/patches/clang.patch', '/plugins/non-existent-file') + test.run() + test.expect('Error: debian/patches/clang.patch: Source file does not exist: plugins/non-existent-file') + test.expect('1 errors') + + with ScriptTest(check_strings) as test: + create_file('debian/lmms-common.docs', '/plugins/caps.html') + test.run() + test.expect('Error: debian/lmms-common.docs: Path does not exist: /plugins/caps.html') + test.expect('1 errors') + + with ScriptTest(check_strings) as test: + create_file('debian/copyright', 'Files: NonExistent') + test.run() + test.expect('Error: debian/copyright: Glob/Path does not exist: NonExistent') + test.expect('1 errors') + +# if we made it until here without an exception, all tests have been passed +print("SUCCESS")