I209: Display team stderr to judges #907
Conversation
…e and return the list
There was a problem hiding this comment.
Code looks good, and the PR works as described.
Further testing (than mentioned in the PR) was performed and a couple of issues were found (not related directly to the fixes made by this PR, but it makes sense to include these fixes in the PR as CI):
- Judge the run as "Yes" (Accepted).
- Select the accepted run from the "All Runs" tab.
- Select "Rejudge".
- Select "View Outputs & Data"
One would expect you could select "View" under "Team Output" or "Team StdErr" and see the corresponding outputs. In reality what happens is a NPE exception is generated in TestResultsPane.getFileForTableCell() because "currentTeamOutputFileNames" is null. The tests are wrong on lines 2199:
if (currentTeamOutputFileNames != null || currentTeamOutputFileNames.length >= row) {
and 2209:
if (currentTeamStderrFileNames != null || currentTeamStderrFileNames.length >= row) {
the || should be an &&. If the member is null, it will try to get "null.length" which is an NPE. This code was always there for the currentTeamOutputFileNames case. Both of these cases should be fixed as a CI.
But, the real problem is WHY are those null in the first place (they shouldn't be). The reason appears to be another problem that has always been there. Executable goes out of its way to create the stdout and stderr file lists, but it never saves them in the ExecutionData for the judgement. See class RunResultsFile. The methods setExeuteStdoutFile() and setExecuteStderrFile() are never called to actually set the information to be saved in executionData. This is a legacy bug apparently. This can be fixed as well under a CI with this PR IMHO.
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException at edu.csus.ecs.pc2.ui.TestResultsPane.getFileForTableCell(TestResultsPane.java:2209) at edu.csus.ecs.pc2.ui.TestResultsPane.viewFile(TestResultsPane.java:2065) at edu.csus.ecs.pc2.ui.TestResultsPane$19.mouseClicked(TestResultsPane.java:1748) at java.awt.AWTEventMulticaster.mouseClicked(AWTEventMulticaster.java:270) at java.awt.Component.processMouseEvent(Component.java:6542) at javax.swing.JComponent.processMouseEvent(JComponent.java:3324) at java.awt.Component.processEvent(Component.java:6304) at java.awt.Container.processEvent(Container.java:2239) at java.awt.Component.dispatchEventImpl(Component.java:4889) at java.awt.Container.dispatchEventImpl(Container.java:2297) at java.awt.Component.dispatchEvent(Component.java:4711) at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4904) at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4544) at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4476) at java.awt.Container.dispatchEventImpl(Container.java:2283) at java.awt.Window.dispatchEventImpl(Window.java:2746) at java.awt.Component.dispatchEvent(Component.java:4711) at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:760) at java.awt.EventQueue.access$500(EventQueue.java:97) at java.awt.EventQueue$3.run(EventQueue.java:709) at java.awt.EventQueue$3.run(EventQueue.java:703) at java.security.AccessController.doPrivileged(Native Method) at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74) at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:84) at java.awt.EventQueue$4.run(EventQueue.java:733) at java.awt.EventQueue$4.run(EventQueue.java:731) at java.security.AccessController.doPrivileged(Native Method) at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74) at java.awt.EventQueue.dispatchEvent(EventQueue.java:730) at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205) at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116) at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93) at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)
Great catch, John. I completely agree these issues can and should be fixed as part of this PR, under the banner of "Continuous Improvement". |
There was a problem hiding this comment.
Discussed with @SamanwaySadhu the one CI that I asked about, namely, just setting the output/stderr files so they are preserved. That will not work, it is more involved than that, and out of scope of this PR. As such, as separate issue was be created (#916).
Description of what the PR does
Executableclass to now save Team stderr for each test case in a file and return the list of filesSelectJudgmentPaneNewto receive list of team stderr files from executable and forward it toTestResultsPaneTestResultsFrameto forward list of team stderr files toTestResultsPaneTestResultsPaneto add a new column for team stderr and show the stderr upon button clickTestCaseResultsTableModelto handle new column for team stderrTestResultsRowDatato handle new column for team stderIssue which the PR addresses
Fixes #209
Environment in which the PR was developed (OS, IDE, Java version, etc.)
Windows 10, Eclipse 2021-12 R, JDK 8u381 (1.8), C++ 13.2.0
Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)
Problemspane make sure one of the problems have judging type set to Manual, if not edit and set to Manual.Auto Judgepane make sure one of the judges have Problems(none selected), if not edit and set.cerr. Need to be tested for other languages.New Runspane and request run.Select Judgement for Run {:id}window click on Execute Run.Test Results for Run {:id}you should see a new column nameTeam StdErr. Click on View to view the StdErr.Additional Context
currentTeamOutputFileNames != null || currentTeamOutputFileNames.length >= rowand because the condition has OR it checks for the length even if the variable is null.Resolution: Changed the condition to AND i.e.
&&.executesiteNjudgeMand the contents are deleted each timeSelectJudgementFrameis called. Currently a testcase output and stderr is being stored in a SerializedFile in ExecutionData object of Executable class and it gets overwritten for each test case (can be resolved by making it a list of SerilizedFiles). However the Executable/ExecutionData object is not saved anywhere in PC2 database. The TestResultsPane table is being populated using objects of Run, RunFiles, RunTestCase (it stores only the verdict) but they do not store the actual output/stderr.Resolution: Need to be researched more. It is a potential roadblock to solving issues Provide a way for a judge to re-execute/judge a single test case #35, On Judge UI add feature to execute all test cases if stop-on-first-failed-test-case is true #39, etc.