Pretest dependencies #1075
Pretest dependencies #1075
Conversation
|
We'll need an accompanying site-side PR to not crash the bridge when we send it |
Riolku
force-pushed
the
pretest-dependencies
branch
from
8da87e8
to
c91c9d8
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
c91c9d8
to
2a9d3e7
Compare
|
Also have |
Riolku
force-pushed
the
pretest-dependencies
branch
from
2a9d3e7
to
0e49c23
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
0e49c23
to
d68e6b2
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
d68e6b2
to
4e31149
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
4e31149
to
e3b2ea6
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
e3b2ea6
to
db2071c
Compare
Codecov ReportBase: 80.75% // Head: 53.87% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #1075 +/- ##
===========================================
- Coverage 80.75% 53.87% -26.88%
===========================================
Files 134 134
Lines 4770 4850 +80
===========================================
- Hits 3852 2613 -1239
- Misses 918 2237 +1319
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Riolku
force-pushed
the
pretest-dependencies
branch
2 times, most recently
from
fe77c24
to
1fe64ba
Compare
Riolku
force-pushed
the
pretest-dependencies
branch
from
1fe64ba
to
621ce44
Compare
This commit changes the previous "batch dependencies" in a few ways: 1. Dependencies can now be specified on any top-level unit, case or batch. 2. Dependencies are specified with YAML alias notation, not numbers. It also implements pretests, including a `PRETEST-END` IPC packet.
Riolku
force-pushed
the
pretest-dependencies
branch
from
621ce44
to
5001269
Compare
| ) | ||
| else: | ||
| self.main_case += 1 | ||
| report(ansi_style('%sTest case %2d %-3s %s' % (case_padding, self.main_case, colored_codes[0], case_info))) |
There was a problem hiding this comment.
This string is almost identical to the one used for pretests. Can we make "Test case" and "Pretest case" substituted in with another %s?
Same comment below for batches.
| @@ -194,11 +196,25 @@ def _ipc_compile_message(self, _report, compile_message: str) -> None: | |||
| self.packet_manager.compile_message_packet(compile_message) | |||
|
|
|||
| def _ipc_grading_begin(self, _report, is_pretested: bool) -> None: | |||
| self.in_pretests = False | |||
| self.pretest_batch = 0 | |||
There was a problem hiding this comment.
Can we use a single set of batch/case variables, and reset them when we end pretests? That way we don't have dangling state after pretests finish.
| class GradingAbort(Exception): | ||
| pass | ||
|
|
||
| def grade(case: AbstractTestCase) -> Result: |
There was a problem hiding this comment.
| def grade(case: AbstractTestCase) -> Result: | |
| def grade_single(case: AbstractTestCase) -> Result: |
To match the naming of the rest of the functions.
| # output. So, we trim it here so we don't run out of memory in the controller. | ||
| result.proc_output = result.output | ||
| yield IPC.RESULT, (batch_number, case_number, result) | ||
| failed_pretests: Set[int] = set() |
There was a problem hiding this comment.
Rather than have this state here captured by the closure in should_run_main_test, what if we took it as the first parameter to should_run_main_test, and then applied it with partial(...?
| failed_pretests, pretest_result = yield from grade_many(pretests, should_run_pretest) | ||
| yield IPC.PRETEST_END, () | ||
| if not self.grader.run_pretests_only: | ||
| if pretest_result == GradeManyResult.SKIP_ALL: |
There was a problem hiding this comment.
Can this ever happen without len(failed_pretests) != 0? Do we need this extra enum, or does nonzero failed_pretests already imply SKIP_ALL here?
| failed = True | ||
| yield from skip_toplevel(test) | ||
| elif isinstance(test, Batch): | ||
| yield IPC.BATCH_BEGIN, (test.batch,) |
There was a problem hiding this comment.
Can we move these yields into grade_batched_cases? It's a little weird that the case results come from within it, but the batch begin/end markers don't.
| result = self.grader.grade(case) | ||
| result = grade(test) | ||
| failed = result.result_flag & Result.WA | ||
| yield IPC.RESULT, (None, test.position, result) |
There was a problem hiding this comment.
Can we move this yield into grade?
This PR changes the previous "batch dependencies" in a few ways:
It also implements pretests, including a
PRETEST-ENDIPC packet.