[TestcaseGroup problems] Extend testcases UI page & exporting of problems - #3623
Open
vmcj wants to merge 20 commits into
Open
[TestcaseGroup problems] Extend testcases UI page & exporting of problems#3623vmcj wants to merge 20 commits into
vmcj wants to merge 20 commits into
Conversation
When a new testcase was added we would also update the description with the empty string as it was `null` before.
Since the additions of the testcase groups we would switch for every tr, not for only the testcase rows. This changes the selector to do the right thing again.
…rectory This can be checked with the addone problem where we would put the sample testcases not in it's own group in the interface but only the secret data. Verified that this works by checking both addone (where sample should now be under the root) and hello where we don't want to display anything.
This can be tested with having 1.desc, the description for testcase 1. We all use unix line endings, but the http spec wants \r\n endings. So when you save the testcases we would update testcase 1 but nothing (really) changed.
Alternative is to always fill the description with the empty string and make it non-nullable.
There are some issues with floats/integer conversions but these seem to work on a quick glance.
This reverts commit 161fafd.
This reverts commit 1611a47.
vmcj
force-pushed
the
output_validator_args
branch
from
May 4, 2026 16:13
b95f9bf to
5bb28b3
Compare
meisterT
reviewed
Jun 6, 2026
meisterT
left a comment
meisterT
left a comment
Member
There was a problem hiding this comment.
there are some phpcs nits as well
| $testdataConfig['output_validator_flags'] = $testcaseGroup->getOutputValidatorFlags(); | ||
| } | ||
| if ($testcaseGroup->getRangeLowerBound()) { | ||
| $testdataConfig['range'] = sprintf("range: %d %d", $testcaseGroup->getRangeLowerBound(), $testcaseGroup->getRangeUpperBound()); |
Member
There was a problem hiding this comment.
does this need the extra 'range: ? Isn't that already given by the key? I think it currently serializes as range: range: 0 100 (or similar)
| // TODO: This is slow but prevents us from doing a transaction | ||
| $this->em->flush(); | ||
| } | ||
| $newTestcase->setRank($last->getRank() + 1); |
Member
There was a problem hiding this comment.
$last can be null here, or am I missing something?
| $testdataConfig['accept_score'] = $testcaseGroup->getAcceptScore(); | ||
| } | ||
| if ($testcaseGroup->getAggregationType() !== TestcaseAggregationType::SUM) { | ||
| $testdataConfig['grader_flags'] = $testcaseGroup->getAggregationType()->name; |
| if ($testcaseGroup->isOnRejectContinue()) { | ||
| $testdataConfig['on_reject'] = 'continue'; | ||
| } | ||
| $zip->addFromString($testcaseDirectory . '/testdata.yaml', Yaml::dump($testdataConfig)); |
Member
There was a problem hiding this comment.
Let's only add it if $testdataConfig is non-empty?
|
|
||
| $formatString = sprintf('data/%%s/%%0%dd', ceil(log10(count($testcases) + 1))); | ||
| $formatString = sprintf('%%s/%%0%dd', ceil(log10(count($testcases) + 1))); | ||
| $rankInGroup = 0; |
Member
There was a problem hiding this comment.
This was already the case before your change, but it looks like $rankInGroup is actually calculating a global rank not a per group rank?
| <label class="col-sm-2 col-form-label" for="add_testgroup">Testgroup</label> | ||
| <div class="col-sm-4"> | ||
| <select class="form-select" name="add_testgroup" id="add_testgroup"> | ||
| {% for testgroup in testcaseGroups %} |
Member
There was a problem hiding this comment.
Do we need to add some default for no group here as well?
| } | ||
| } | ||
| if ($testCaseGroups !== []) { | ||
| foreach (['data/sample', 'data/secret'] as $default_dir) { |
Member
There was a problem hiding this comment.
I don't think this is correct, see https://icpc.io/problem-package-format/spec/2025-09.html#samples-and-full-feedback
Member
Author
There was a problem hiding this comment.
Ok, that was the missing spec piece I missed.
This was referenced Jun 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code might not be great/performant but it fixes a couple of (real) issues, so I propose to be lenient and fix performance issues when they show up.
Will squash after approval & take the commit named testing out.
@meisterT I had to change the scores, I think it makes sense as the samples should be a testgroup but maybe you did this by design and I interpret the spec wrong?