From 1b16738255c528ca9e1da860366ed412364a45dd Mon Sep 17 00:00:00 2001 From: Mateusz Masiarz Date: Sun, 24 Sep 2023 19:54:08 +0200 Subject: [PATCH] Bug fixes for making changes after updating sinol-make (#131) * Fix changing version breaking config * Delete cached tests after contest type change * Add tests * Add tests for removing cache after contest type change * Remove debug * Remove version changes, catch errors while validating expected scores * Add tests * Add TotalPointsChange in validating expected scores * Refactor * Add description for `try_fix_config` function * Refactor * Bump version for release --- src/sinol_make/__init__.py | 3 +- src/sinol_make/commands/run/__init__.py | 40 +++++++-- src/sinol_make/helpers/cache.py | 25 +++++- src/sinol_make/helpers/package_util.py | 23 +++++ src/sinol_make/structs/status_structs.py | 11 +++ src/sinol_make/util.py | 88 +++++++++++-------- tests/commands/run/test_integration.py | 46 +++++++++- tests/commands/run/test_unit.py | 55 ++++++++++-- tests/helpers/test_cache.py | 12 +++ tests/test_util.py | 14 +++ tests/version_changes/__init__.py | 0 .../test_expected_scores_format_change.py | 31 ------- 12 files changed, 260 insertions(+), 88 deletions(-) delete mode 100644 tests/version_changes/__init__.py delete mode 100644 tests/version_changes/test_expected_scores_format_change.py diff --git a/src/sinol_make/__init__.py b/src/sinol_make/__init__.py index e7d33398..00e37c06 100644 --- a/src/sinol_make/__init__.py +++ b/src/sinol_make/__init__.py @@ -9,7 +9,7 @@ from sinol_make import util, oiejq -__version__ = "1.5.9" +__version__ = "1.5.10" def configure_parsers(): @@ -61,7 +61,6 @@ def main_exn(): except Exception as err: util.exit_with_error('`oiejq` could not be installed.\n' + err) - util.make_version_changes() command.run(args) exit(0) diff --git a/src/sinol_make/commands/run/__init__.py b/src/sinol_make/commands/run/__init__.py index 3bc4d649..905e6619 100644 --- a/src/sinol_make/commands/run/__init__.py +++ b/src/sinol_make/commands/run/__init__.py @@ -17,7 +17,8 @@ from sinol_make.interfaces.BaseCommand import BaseCommand from sinol_make.interfaces.Errors import CompilationError, CheckerOutputException, UnknownContestType from sinol_make.helpers import compile, compiler, package_util, printer, paths, cache -from sinol_make.structs.status_structs import Status, ResultChange, PointsChange, ValidationResult, ExecutionResult +from sinol_make.structs.status_structs import Status, ResultChange, PointsChange, ValidationResult, ExecutionResult, \ + TotalPointsChange import sinol_make.util as util import yaml, os, collections, sys, re, math, dictdiffer import multiprocessing as mp @@ -827,6 +828,7 @@ def validate_expected_scores(self, results): added_groups = set() removed_groups = set() changes = [] + unknown_change = False for type, field, change in list(expected_scores_diff): if type == "add": @@ -877,6 +879,16 @@ def validate_expected_scores(self, results): old_result=change[0], result=change[1] )) + elif field[1] == "points": # Points for at least one solution has changed + solution = field[0] + changes.append(TotalPointsChange( + solution=solution, + old_points=change[0], + new_points=change[1] + )) + else: + unknown_change = True + return ValidationResult( added_solutions, @@ -885,7 +897,8 @@ def validate_expected_scores(self, results): removed_groups, changes, expected_scores, - new_expected_scores + new_expected_scores, + unknown_change, ) @@ -893,6 +906,10 @@ def print_expected_scores_diff(self, validation_results: ValidationResult): diff = validation_results config_expected_scores = self.config.get("sinol_expected_scores", {}) + if diff.unknown_change: + print(util.error("There was an unknown change in expected scores. " + "You should apply the suggested changes to avoid errors.")) + def warn_if_not_empty(set, message): if len(set) > 0: print(util.warning(message + ": "), end='') @@ -916,8 +933,11 @@ def print_points_change(solution, group, new_points, old_points): print_points_change(change.solution, change.group, change.result, change.old_result) elif isinstance(change, PointsChange): print_points_change(change.solution, change.group, change.new_points, change.old_points) + elif isinstance(change, TotalPointsChange): + print(util.warning("Solution %s passed all groups with %d points while it should pass with %d points." % + (change.solution, change.new_points, change.old_points))) - if diff.expected_scores == diff.new_expected_scores: + if diff.expected_scores == diff.new_expected_scores and not diff.unknown_change: print(util.info("Expected scores are correct!")) else: def delete_group(solution, group): @@ -935,7 +955,6 @@ def set_group_result(solution, group, result): self.possible_score ) - if self.args.apply_suggestions: for solution in diff.removed_solutions: del config_expected_scores[solution] @@ -951,7 +970,6 @@ def set_group_result(solution, group, result): else: config_expected_scores[solution] = diff.new_expected_scores[solution] - self.config["sinol_expected_scores"] = self.convert_status_to_string(config_expected_scores) util.save_config(self.config) print(util.info("Saved suggested expected scores description.")) @@ -1129,6 +1147,7 @@ def run(self, args): print("Task: %s (tag: %s)" % (title, self.ID)) self.cpus = args.cpus or mp.cpu_count() cache.save_to_cache_extra_compilation_files(self.config.get("extra_compilation_files", []), self.ID) + cache.remove_results_if_contest_type_changed(self.config.get("sinol_contest_type", "default")) checker = package_util.get_files_matching_pattern(self.ID, f'{self.ID}chk.*') if len(checker) != 0: @@ -1158,6 +1177,15 @@ def run(self, args): results, all_results = self.compile_and_run(solutions) self.check_errors(all_results) - validation_results = self.validate_expected_scores(results) + try: + validation_results = self.validate_expected_scores(results) + except: + self.config = util.try_fix_config(self.config) + try: + validation_results = self.validate_expected_scores(results) + except: + util.exit_with_error("Validating expected scores failed. " + "This probably means that `sinol_expected_scores` is broken. " + "Delete it and run `sinol-make run --apply-suggestions` again.") self.print_expected_scores_diff(validation_results) self.exit() diff --git a/src/sinol_make/helpers/cache.py b/src/sinol_make/helpers/cache.py index 5d9a6993..2a2d34e3 100644 --- a/src/sinol_make/helpers/cache.py +++ b/src/sinol_make/helpers/cache.py @@ -67,10 +67,7 @@ def save_compiled(file_path: str, exe_path: str, is_checker: bool = False): info.save(file_path) if is_checker: - for solution in os.listdir(paths.get_cache_path('md5sums')): - info = get_cache_file(solution) - info.tests = {} - info.save(solution) + remove_results_cache() def save_to_cache_extra_compilation_files(extra_compilation_files, task_id): @@ -100,3 +97,23 @@ def save_to_cache_extra_compilation_files(extra_compilation_files, task_id): info.md5sum = md5sum info.save(file_path) + + +def remove_results_cache(): + """ + Removes all cached test results + """ + for solution in os.listdir(paths.get_cache_path('md5sums')): + info = get_cache_file(solution) + info.tests = {} + info.save(solution) + + +def remove_results_if_contest_type_changed(contest_type): + """ + Checks if contest type has changed and removes all cached test results if it has. + :param contest_type: Contest type + """ + if package_util.check_if_contest_type_changed(contest_type): + remove_results_cache() + package_util.save_contest_type_to_cache(contest_type) diff --git a/src/sinol_make/helpers/package_util.py b/src/sinol_make/helpers/package_util.py index 4afd68f9..e4bd8b5e 100644 --- a/src/sinol_make/helpers/package_util.py +++ b/src/sinol_make/helpers/package_util.py @@ -294,3 +294,26 @@ def any_files_matching_pattern(task_id: str, pattern: str) -> bool: :return: True if any file in package matches given pattern. """ return len(get_files_matching_pattern(task_id, pattern)) > 0 + + +def check_if_contest_type_changed(contest_type): + """ + Checks if contest type in cache is different then contest type specified in config.yml. + :param contest_type: Contest type specified in config.yml. + :return: True if contest type in cache is different then contest type specified in config.yml. + """ + if not os.path.isfile(paths.get_cache_path("contest_type")): + return False + with open(paths.get_cache_path("contest_type"), "r") as contest_type_file: + cached_contest_type = contest_type_file.read() + return cached_contest_type != contest_type + + +def save_contest_type_to_cache(contest_type): + """ + Saves contest type to cache. + :param contest_type: Contest type. + """ + os.makedirs(paths.get_cache_path(), exist_ok=True) + with open(paths.get_cache_path("contest_type"), "w") as contest_type_file: + contest_type_file.write(contest_type) diff --git a/src/sinol_make/structs/status_structs.py b/src/sinol_make/structs/status_structs.py index d3f63c70..2ff3ce11 100644 --- a/src/sinol_make/structs/status_structs.py +++ b/src/sinol_make/structs/status_structs.py @@ -37,6 +37,10 @@ def from_str(status): else: raise ValueError(f"Unknown status: '{status}'") + @staticmethod + def possible_statuses(): + return [Status.PENDING, Status.CE, Status.TL, Status.ML, Status.RE, Status.WA, Status.OK] + @dataclass class ResultChange: @@ -46,6 +50,12 @@ class ResultChange: result: Status +@dataclass +class TotalPointsChange: + solution: str + old_points: int + new_points: int + @dataclass class PointsChange: solution: str @@ -63,6 +73,7 @@ class ValidationResult: changes: List[ResultChange] expected_scores: dict new_expected_scores: dict + unknown_change: bool @dataclass diff --git a/src/sinol_make/util.py b/src/sinol_make/util.py index dfd81005..a0fb7173 100644 --- a/src/sinol_make/util.py +++ b/src/sinol_make/util.py @@ -10,6 +10,7 @@ import sinol_make from sinol_make.contest_types import get_contest_type +from sinol_make.structs.status_structs import Status def get_commands(): @@ -87,7 +88,7 @@ def save_config(config): { "key": "sinol_expected_scores", "default_flow_style": None - } + }, ] config = config.copy() @@ -293,41 +294,56 @@ def get_file_md5(path): return hashlib.md5(f.read()).hexdigest() -def make_version_changes(): - if compare_versions(sinol_make.__version__, "1.5.8") == 1: - # In version 1.5.9 we changed the format of sinol_expected_scores. - # Now all groups have specified points and status. - - if find_and_chdir_package(): - with open("config.yml", "r") as config_file: - config = yaml.load(config_file, Loader=yaml.FullLoader) - - try: - new_expected_scores = {} - expected_scores = config["sinol_expected_scores"] - contest = get_contest_type() - groups = [] - for solution, results in expected_scores.items(): - for group in results["expected"].keys(): - if group not in groups: - groups.append(int(group)) - - scores = contest.assign_scores(groups) - for solution, results in expected_scores.items(): - new_expected_scores[solution] = {"expected": {}, "points": results["points"]} - for group, result in results["expected"].items(): - new_expected_scores[solution]["expected"][group] = {"status": result} - if result == "OK": - new_expected_scores[solution]["expected"][group]["points"] = scores[group] - else: - new_expected_scores[solution]["expected"][group]["points"] = 0 - config["sinol_expected_scores"] = new_expected_scores - save_config(config) - except: - # If there is an error, we just delete the field. - if "sinol_expected_scores" in config: - del config["sinol_expected_scores"] - save_config(config) +def try_fix_config(config): + """ + Function to try to fix the config.yml file. + Tries to: + - reformat `sinol_expected_scores` field + :param config: config.yml file as a dict + :return: config.yml file as a dict + """ + # The old format was: + # sinol_expected_scores: + # solution1: + # expected: {1: OK, 2: OK, ...} + # points: 100 + # + # We change it to: + # sinol_expected_scores: + # solution1: + # expected: {1: {status: OK, points: 100}, 2: {status: OK, points: 100}, ...} + # points: 100 + try: + new_expected_scores = {} + expected_scores = config["sinol_expected_scores"] + contest = get_contest_type() + groups = [] + for solution, results in expected_scores.items(): + for group in results["expected"].keys(): + if group not in groups: + groups.append(int(group)) + + scores = contest.assign_scores(groups) + for solution, results in expected_scores.items(): + new_expected_scores[solution] = {"expected": {}, "points": results["points"]} + for group, result in results["expected"].items(): + if result in Status.possible_statuses(): + new_expected_scores[solution]["expected"][group] = {"status": result} + if result == "OK": + new_expected_scores[solution]["expected"][group]["points"] = scores[group] + else: + new_expected_scores[solution]["expected"][group]["points"] = 0 + else: + # This means that the result is probably valid. + new_expected_scores[solution]["expected"][group] = result + config["sinol_expected_scores"] = new_expected_scores + save_config(config) + except: + # If there is an error, we just delete the field. + if "sinol_expected_scores" in config: + del config["sinol_expected_scores"] + save_config(config) + return config def color_red(text): return "\033[91m{}\033[00m".format(text) diff --git a/tests/commands/run/test_integration.py b/tests/commands/run/test_integration.py index c879bb6e..2fc4fce4 100644 --- a/tests/commands/run/test_integration.py +++ b/tests/commands/run/test_integration.py @@ -22,13 +22,20 @@ def test_simple(create_package, time_tool): """ package_path = create_package create_ins_outs(package_path) - parser = configure_parsers() + with open(os.path.join(os.getcwd(), "config.yml"), "r") as config_file: + config = yaml.load(config_file, Loader=yaml.SafeLoader) + expected_scores = config["sinol_expected_scores"] + args = parser.parse_args(["run", "--time-tool", time_tool]) command = Command() command.run(args) + with open(os.path.join(os.getcwd(), "config.yml"), "r") as config_file: + config = yaml.load(config_file, Loader=yaml.SafeLoader) + assert config["sinol_expected_scores"] == expected_scores + @pytest.mark.parametrize("create_package", [get_simple_package_path(), get_verify_status_package_path(), get_checker_package_path(), get_library_package_path(), @@ -634,6 +641,43 @@ def test(file_to_change, lang, comment_character): test("liblib.py", "py", "#") +@pytest.mark.parametrize("create_package", [get_simple_package_path()], indirect=True) +def test_contest_type_change(create_package, time_tool): + """ + Test if after changing contest type, all cached test results are removed. + """ + package_path = create_package + create_ins_outs(package_path) + parser = configure_parsers() + args = parser.parse_args(["run", "--time-tool", time_tool]) + command = Command() + + # First run to cache test results. + command.run(args) + + # Change contest type. + config_path = os.path.join(os.getcwd(), "config.yml") + with open(config_path, "r") as f: + config = yaml.load(f, Loader=yaml.SafeLoader) + config["sinol_contest_type"] = "oi" + with open(config_path, "w") as f: + f.write(yaml.dump(config)) + + # Compile checker check if test results are removed. + command = Command() + # We remove tests, so that `run()` exits before creating new cached test results. + for test in glob.glob("in/*.in"): + os.unlink(test) + with pytest.raises(SystemExit): + command.run(args) + + task_id = package_util.get_task_id() + solutions = package_util.get_solutions(task_id, None) + for solution in solutions: + cache_file: CacheFile = cache.get_cache_file(solution) + assert cache_file.tests == {} + + @pytest.mark.parametrize("create_package", [get_simple_package_path()], indirect=True) def test_cwd_in_prog(create_package): """ diff --git a/tests/commands/run/test_unit.py b/tests/commands/run/test_unit.py index 5e2d2a87..28e770ab 100644 --- a/tests/commands/run/test_unit.py +++ b/tests/commands/run/test_unit.py @@ -96,7 +96,7 @@ def test_validate_expected_scores_success(): } results = command.validate_expected_scores(results) assert results.expected_scores != results.new_expected_scores - assert len(results.changes) == 2 + assert len(results.changes) == 3 # Test with removed solution. command.args = argparse.Namespace(solutions=None, tests=None, print_expected_scores=True) @@ -181,7 +181,8 @@ def test_print_expected_scores_diff(capsys, create_package): new_expected_scores={ "abc.cpp": {1: {"status": "OK", "points": 100}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) command.print_expected_scores_diff(results) out = capsys.readouterr().out @@ -201,7 +202,8 @@ def test_print_expected_scores_diff(capsys, create_package): new_expected_scores={ "abc.cpp": {1: {"status": "WA", "points": 0}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) with pytest.raises(SystemExit) as e: command.print_expected_scores_diff(results) @@ -226,7 +228,8 @@ def test_print_expected_scores_diff(capsys, create_package): 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, "abc5.cpp": {1: {"status": "OK", "points": 100}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) with pytest.raises(SystemExit) as e: command.print_expected_scores_diff(results) @@ -251,7 +254,8 @@ def test_print_expected_scores_diff(capsys, create_package): new_expected_scores={ "abc.cpp": {1: {"status": "OK", "points": 100}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) with pytest.raises(SystemExit) as e: command.print_expected_scores_diff(results) @@ -275,7 +279,8 @@ def test_print_expected_scores_diff(capsys, create_package): "abc.cpp": {1: {"status": "OK", "points": 100}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}, 5: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) with pytest.raises(SystemExit) as e: command.print_expected_scores_diff(results) @@ -299,7 +304,8 @@ def test_print_expected_scores_diff(capsys, create_package): new_expected_scores={ "abc.cpp": {1: {"status": "OK", "points": 100}, 2: {"status": "OK", "points": 100}, 3: {"status": "OK", "points": 100}, 4: {"status": "OK", "points": 100}}, - } + }, + unknown_change=False, ) with pytest.raises(SystemExit) as e: command.print_expected_scores_diff(results) @@ -341,7 +347,8 @@ def test_print_expected_scores_diff(capsys, create_package): 3: {"status": "OK", "points": 25}, 5: {"status": "OK", "points": 0}}, "points": 75 } - } + }, + unknown_change=False, ) command.print_expected_scores_diff(results) out = capsys.readouterr().out @@ -382,6 +389,38 @@ def test_print_expected_scores_diff(capsys, create_package): } } + # Test with `unknown_changes = True` + command.args = argparse.Namespace(apply_suggestions=False) + results = ValidationResult( + added_solutions=set(), + removed_solutions=set(), + added_groups=set(), + removed_groups=set(), + changes=[], + expected_scores={ + "abc.cpp": { + "expected": {1: {"status": "OK", "points": 25}, 2: {"status": "OK", "points": 25}, + 3: {"status": "OK", "points": 25}, 4: {"status": "OK", "points": 25}}, + "points": 100 + }, + }, + new_expected_scores={ + "abc.cpp": { + "expected": {1: {"status": "OK", "points": 25}, 2: {"status": "OK", "points": 25}, + 3: {"status": "OK", "points": 25}, 4: {"status": "OK", "points": 25}}, + "points": 100 + }, + }, + unknown_change=True, + ) + with pytest.raises(SystemExit) as e: + command.print_expected_scores_diff(results) + out = capsys.readouterr().out + assert e.type == SystemExit + assert e.value.code == 1 + assert "There was an unknown change in expected scores." in out + + @pytest.mark.parametrize("create_package", [get_simple_package_path()], indirect=True) def test_set_scores(create_package): diff --git a/tests/helpers/test_cache.py b/tests/helpers/test_cache.py index 9f573065..9d9fe719 100644 --- a/tests/helpers/test_cache.py +++ b/tests/helpers/test_cache.py @@ -94,3 +94,15 @@ def test_cache(): assert os.path.exists(paths.get_cache_path("md5sums", "abc.py")) assert cache.get_cache_file("abc.py") == cache_file assert cache.get_cache_file("abc.cpp") == CacheFile() + + # Test if after changing contest type all cached test results are removed + cache_file.save("abc.cpp") + cache_file.save("abc.py") + + cache.remove_results_if_contest_type_changed("default") + assert cache.get_cache_file("abc.py") == cache_file + assert cache.get_cache_file("abc.cpp") == cache_file + + cache.remove_results_if_contest_type_changed("oi") + assert cache.get_cache_file("abc.py").tests == {} + assert cache.get_cache_file("abc.cpp").tests == {} diff --git a/tests/test_util.py b/tests/test_util.py index 8e06ea06..ac674140 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -8,6 +8,7 @@ import resource import requests_mock import pytest +import yaml from sinol_make import util, configure_parsers from tests import util as test_util @@ -95,3 +96,16 @@ def test_check_version(**kwargs): mocker.get("https://pypi.python.org/pypi/sinol-make/json", exc=requests.exceptions.ConnectTimeout) util.check_version() assert not version_file.is_file() + + +@pytest.mark.parametrize("create_package", [test_util.get_simple_package_path()], indirect=True) +def test_version_change(create_package): + with open("config.yml", "r") as config_file: + config = yaml.load(config_file, Loader=yaml.FullLoader) + old_expected_scores = config["sinol_expected_scores"] + config["sinol_expected_scores"] = {'abc.cpp': {'points': 100, 'expected': {1: 'OK', 2: 'OK', 3: 'OK', 4: 'OK'}}, + 'abc1.cpp': {'points': 75, 'expected': {1: 'OK', 2: 'OK', 3: 'OK', 4: 'WA'}}, + 'abc2.cpp': {'points': 25, 'expected': {1: 'OK', 2: 'WA', 3: 'WA', 4: 'TL'}}, + 'abc3.cpp': {'points': 25, 'expected': {1: 'OK', 2: 'WA', 3: 'WA', 4: 'ML'}}, + 'abc4.cpp': {'points': 50, 'expected': {1: 'OK', 2: 'OK', 3: 'WA', 4: 'RE'}}} + assert old_expected_scores == util.try_fix_config(config)["sinol_expected_scores"] diff --git a/tests/version_changes/__init__.py b/tests/version_changes/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/version_changes/test_expected_scores_format_change.py b/tests/version_changes/test_expected_scores_format_change.py deleted file mode 100644 index 9a721807..00000000 --- a/tests/version_changes/test_expected_scores_format_change.py +++ /dev/null @@ -1,31 +0,0 @@ -import pytest -import yaml - -from tests import util -from tests.fixtures import create_package -from sinol_make.util import make_version_changes -import sinol_make - - -@pytest.mark.parametrize("create_package", [util.get_simple_package_path()], indirect=True) -def test_version_change(create_package): - orig_version = sinol_make.__version__ - sinol_make.__version__ = "1.5.9" - - with open("config.yml", "r") as config_file: - config = yaml.load(config_file, Loader=yaml.FullLoader) - old_expected_scores = config["sinol_expected_scores"] - config["sinol_expected_scores"] = {'abc.cpp': {'points': 100, 'expected': {1: 'OK', 2: 'OK', 3: 'OK', 4: 'OK'}}, - 'abc1.cpp': {'points': 75, 'expected': {1: 'OK', 2: 'OK', 3: 'OK', 4: 'WA'}}, - 'abc2.cpp': {'points': 25, 'expected': {1: 'OK', 2: 'WA', 3: 'WA', 4: 'TL'}}, - 'abc3.cpp': {'points': 25, 'expected': {1: 'OK', 2: 'WA', 3: 'WA', 4: 'ML'}}, - 'abc4.cpp': {'points': 50, 'expected': {1: 'OK', 2: 'OK', 3: 'WA', 4: 'RE'}}} - with open("config.yml", "w") as config_file: - yaml.dump(config, config_file) - - make_version_changes() - - with open("config.yml", "r") as config_file: - config = yaml.load(config_file, Loader=yaml.FullLoader) - assert config["sinol_expected_scores"] == old_expected_scores - sinol_make.__version__ = orig_version