Testresults module: form-based parameter binding, UserSchema registration, and Selenium tests#622
Draft
vagisha wants to merge 9 commits intorelease26.3-SNAPSHOTfrom
Draft
Testresults module: form-based parameter binding, UserSchema registration, and Selenium tests#622vagisha wants to merge 9 commits intorelease26.3-SNAPSHOTfrom
vagisha wants to merge 9 commits intorelease26.3-SNAPSHOTfrom
Conversation
- Register JQuery CDN for CSS and images - TestResultsSchema extends UserSchema so we don't have to add it as external schema. Add FK lookup columns for tables - Override addNavTrail so we don't get missing page title warnings when running tests - First version of TestResultsTest. Added example XML files.
…nding - Replaced manual request.getParameter() calls with form classes for all actions that accept parameters. - Use Integer/Boolean instead of primitives for optional params to enable null detection. - Added ParseException handling with SimpleErrorView for date parameters. - Added null guards with descriptive error messages for required parameters. - Added cause field to error API responses and updated JSP alert messages to display it. - Replaced deprecated javax.management.modelmbean.XMLParseException with IllegalArgumentException. - Switched from org.springframework.util.StringUtils to org.apache.commons.lang3.StringUtils.
…serialize DOM content. Replaced with Transformer to properly serialize the XML before compressing and storing in the database. - Renamed "user" URL parameter to "username". "user" is in LabKey's disallowed list (HasAllowBindParameter), so the form field was always null, causing ShowUserAction to ignore the selected user and return all runs. - Added sample data for a second computer. Expanded XML files to 150 tests with realistic timestamps and memory profiles to support trend chart rendering. - Expanded Selenium test coverage
There was a problem hiding this comment.
Pull request overview
This PR modernizes the testresults module by switching controllers to Spring form-based parameter binding, registering TestResultsSchema as a UserSchema (so tables are visible/navigable in the query browser), and adding Selenium coverage + sample XML data to validate key UI workflows.
Changes:
- Register
TestResultsSchemaviaDefaultSchema.registerProvider()and enhance schema browsing with UserSchema-level FK resolution. - Replace manual
request.getParameter()parsing across multiple actions with typed form classes, adding better date parsing error handling and consistentSuccess/causeAPI responses. - Add a new end-to-end Selenium test suite plus sample XML inputs covering begin/run/user/failures/flags/training/log/xml/boundaries/user-active/delete flows.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java | New Selenium test suite that posts sample XML and exercises major UI + API flows. |
| testresults/test/sampledata/testresults/pc1-run-0114-disposable.xml | Sample run used for delete-run workflow test. |
| testresults/test/sampledata/testresults/pc1-run-0115-clean.xml | Sample clean run (includes <Log>) used for training/log/xml tests. |
| testresults/test/sampledata/testresults/pc1-run-0116-failures.xml | Sample run with failures for failure-detail and summary tests. |
| testresults/test/sampledata/testresults/pc1-run-0117-leaks.xml | Sample run with memory + handle leaks for leak summaries/tests. |
| testresults/test/sampledata/testresults/pc2-run-0115-clean.xml | Second computer clean run to validate multi-user summaries. |
| testresults/test/sampledata/testresults/pc2-run-0116-failures.xml | Second computer failures run to validate aggregate failure counts. |
| testresults/test/sampledata/testresults/pc2-run-0117-leaks.xml | Second computer leaks run to validate aggregate leak counts/means. |
| testresults/src/org/labkey/testresults/view/user.jsp | Align “username” parameter naming and improve training-set error alerts using cause. |
| testresults/src/org/labkey/testresults/view/trainingdata.jsp | Align “username” parameter naming, improve alerts, and update retrain-all response key to Success. |
| testresults/src/org/labkey/testresults/view/rundown.jsp | Improve training-set error alert messaging to include cause. |
| testresults/src/org/labkey/testresults/view/runDetail.jsp | Align runId param casing for ViewLog/ViewXml + “username” links + improved training-set error alerts. |
| testresults/src/org/labkey/testresults/view/failureDetail.jsp | Update tablesorter configuration for the problem column. |
| testresults/src/org/labkey/testresults/TestResultsWebPart.java | Update webpart to call new getRunDownBean(user, container) overload. |
| testresults/src/org/labkey/testresults/TestResultsSchema.java | Convert schema to UserSchema, register provider, list tables, and rebind intra-schema FKs for query browser navigation. |
| testresults/src/org/labkey/testresults/TestResultsModule.java | Register schema provider in init; add CSP allowlisting for jQuery UI CSS/images. |
| testresults/src/org/labkey/testresults/TestResultsController.java | Introduce form classes, typed param binding, improved date parsing errors, consistent Success keys, and improved XML serialization. |
| testresults/resources/schemas/testresults.xml | Add FK metadata so wrapAllColumns() yields lookup columns that can be converted into UserSchema-level FKs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java
Show resolved
Hide resolved
testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java
Outdated
Show resolved
Hide resolved
testresults/test/src/org/labkey/test/tests/testresults/TestResultsTest.java
Show resolved
Hide resolved
testresults/src/org/labkey/testresults/TestResultsController.java
Outdated
Show resolved
Hide resolved
- TestResultsTest.postSampleXml: parse JSON and assert Success=true instead of substring-matching the response body - TestResultsTest.testBeginPage: start at 01/17/2026 via URL (avoids iterating the datepicker to find the sample-data dates), then navigate via the begin page's <<< / >>> day links. Added - TrainRunAction: restore explicit validation of the `train` param (true/false/force) that was lost in the form-binding refactor. Uses null-safe Strings.CI.equals. - failureDetail.jsp: tablesorter 2.0.5b silently ignored the "#col-problem" headers key (it requires numeric indices). Look up the column index from the ID at init time. Also restored sortList / sortAppend defaults dropped earlier in this branch.
…ge (date-only, DST-safe); add second path via rundown Fail/Leak/Hang matrix - testTrainingDataPage: drop brittle empty-state stats-row assertion - testViewLog: spot-check header and tests from start/middle/end - testDeleteRun: resubmit deleted run ID to confirm it's gone - testChangeBoundaries: add empty-value error-message cases - failureDetail.jsp: skip tablesorter initial sort when tbody is empty - Renamed "cause" to "error" in API response. - Added testApiErrorResponses
- Updated comment for testTrainingDataPage
…at in the error message.
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
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.
Rationale
The
testresultsmodule had no automated tests and used manual request.getParameter() parsing throughout the controller. This PR adds Selenium tests, updates the controller to use form-based parameter binding (replacing manual request.getParameter() calls), and registersTestResultsSchemaas aUserSchemaso that the tables are accessible through the LabKey query schema browser.Changes
TestResultsSchemaextends UserSchema with DefaultSchema.registerProvider() registration, FK lookup columns, and wrapAllColumns() in createTable()causefield added to error API responses; JSP alert messages updated to display itTestResultsTestSelenium test class covering begin page, show run, run lookup, user runs, long-term trends, failures, flagging, and training data workflowsBeginAction(rundown page, date navigation, viewType selector)ShowRunAction(run detail, pass sorting)ShowUserAction(user page, date range)LongTermAction(viewType selector)ShowFailuresAction(failure detail, viewType selector)FlagRunAction/ShowFlaggedAction(flag/unflag workflow)TrainRunAction/TrainingDataViewAction(add/remove training set workflow)ViewLogAction(view run log)ViewXMLAction(view run XML)ChangeBoundariesAction(update warning/error boundaries)SetUserActiveAction(activate/deactivate user)DeleteRunAction(delete a run)javax.management.modelmbean.XMLParseExceptionwithIllegalArgumentExceptionorg.springframework.util.StringUtilstoorg.apache.commons.lang3.StringUtils