Skip to content

Add default test for output validator flags in problem.yaml & testdata.yaml + display - #3655

Merged
vmcj merged 4 commits into
DOMjudge:mainfrom
vmcj:pr3623_extraction_2
Aug 30, 2026
Merged

Add default test for output validator flags in problem.yaml & testdata.yaml + display#3655
vmcj merged 4 commits into
DOMjudge:mainfrom
vmcj:pr3623_extraction_2

Conversation

@vmcj

@vmcj vmcj commented Jun 8, 2026

Copy link
Copy Markdown
Member
image

Extracted some parts of #3623 but also extended from there with the usage of the options in the testcase group.

This adds:

  • the options to a default problem
  • usage in the custom validator
  • unit tests for usage
  • display of those options in the testcases UI.

@vmcj
vmcj force-pushed the pr3623_extraction_2 branch 4 times, most recently from 96bdb91 to db1baa7 Compare June 8, 2026 18:44
@vmcj vmcj changed the title Pr3623 extraction 2 Add default test for output validator flags in problem.yaml & testdata.yaml + display Jun 8, 2026
@vmcj
vmcj force-pushed the pr3623_extraction_2 branch 3 times, most recently from 0562d1d to 93a1267 Compare June 10, 2026 09:05
$orig_compare_args = [];
if ($compare_args !== null && strlen($compare_args) > 0) {
$orig_compare_args = explode(' ', $compare_args);
$orig_compare_args = str_getcsv($compare_args, separator: ' ', escape: '');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might still not what we want, e.g. --flag1 --flag2 (double spaces in between and trailing space) is parsed into ['--flag1', '', '--flag2', ''], isn't it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vmcj vmcj Jun 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don´t implement 2025-09 but https://icpc.io/problem-package-format/spec/legacy.html#test-data-groups where this is a string but only if itś the name. So I suspect we need some extra work here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to figure what spec will be used for the next challenge...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge this for now and open another PR to fix the string vs map of string vs sequence of strings

I'm fine with working on that, I just prefer to not have to further extend this PR with extra features.

}
if (isset($yamlData['output_validator_flags'])) {
$testcaseGroup->setOutputValidatorFlags($yamlData['output_validator_flags']);
foreach(['flags', 'args'] as $postFix) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check whether the user specifies both flags and treat is as an error if so? (same in the other commit, also consider linking to the spec in the corresponding commit messages)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test which checks for this edgecase,

Comment thread webapp/templates/jury/problem_testcases.html.twig
Comment thread example_problems/hangman/output_validator/validator.py Outdated
@vmcj
vmcj force-pushed the pr3623_extraction_2 branch from 93a1267 to 68fcd07 Compare July 3, 2026 17:33
@vmcj
vmcj requested a review from meisterT July 4, 2026 10:30
vmcj added 4 commits August 30, 2026 13:18
Before we would split:
'--message "my message"'
as
['--message', '"my', 'message"']
It's easy to misremember the spec and provide the wrong ones. This follows
the principle that we want to be lenient when importing.
@vmcj
vmcj force-pushed the pr3623_extraction_2 branch from 68fcd07 to 1b32e9b Compare August 30, 2026 11:19
@vmcj
vmcj enabled auto-merge August 30, 2026 11:20
@vmcj
vmcj added this pull request to the merge queue Aug 30, 2026
Merged via the queue into DOMjudge:main with commit 6c33c5c Aug 30, 2026
54 checks passed
@vmcj
vmcj deleted the pr3623_extraction_2 branch August 30, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants