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.
This commit is contained in:
Johannes Lorenz
6
.github/workflows/checks.yml
vendored
6
.github/workflows/checks.yml
vendored
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
@@ -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")
|
||||
24
tests/scripted/README.md
Normal file
24
tests/scripted/README.md
Normal file
@@ -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.
|
||||
|
||||
162
tests/scripted/verify
Executable file
162
tests/scripted/verify
Executable file
@@ -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',
|
||||
'<?xml version="1.0" ?><!DOCTYPE TS><TS language="de" version="2.1">\n'
|
||||
' <context>\n'
|
||||
' <name>TestClass</name>\n'
|
||||
' <message>\n'
|
||||
' <location filename="../../src/core/classes.cpp" line="20"/>\n'
|
||||
' <source>About LMMS</source>\n'
|
||||
' <translation>Über LMMS</translation>\n'
|
||||
' </message>\n'
|
||||
'</context>\n'
|
||||
'</TS>\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',
|
||||
'<?xml version="1.0" ?><!DOCTYPE TS><TS language="de" version="2.1">\n'
|
||||
' <context>\n'
|
||||
' <name>TestClass</name>\n'
|
||||
' <message>\n'
|
||||
' <location filename="../../src/core/non-existent.cpp" line="20"/>\n'
|
||||
' <source>About LMMS</source>\n'
|
||||
' <translation>À propos de LMMS</translation>\n'
|
||||
' </message>\n'
|
||||
'</context>\n'
|
||||
'</TS>\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',
|
||||
'<?xml version="1.0" ?><!DOCTYPE TS><TS language="de" version="2.1">\n'
|
||||
' <context>\n'
|
||||
' <name>NonExistentClass</name>\n'
|
||||
' <message>\n'
|
||||
' <location filename="../../src/core/classes.cpp" line="20"/>\n'
|
||||
' <source>About LMMS</source>\n'
|
||||
' <translation>À propos de LMMS</translation>\n'
|
||||
' </message>\n'
|
||||
'</context>\n'
|
||||
'</TS>\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")
|
||||
Reference in New Issue
Block a user