-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test results visualization #360
base: dev_main
Are you sure you want to change the base?
Conversation
src/main/java/com/mathworks/ci/utilities/MatlabCommandRunner.java
Outdated
Show resolved
Hide resolved
@mw-hrastega, for text review you can go through all the |
src/main/resources/com/mathworks/ci/TestResultsViewAction/index.jelly
Outdated
Show resolved
Hide resolved
<button id="Incomplete" onclick="showTests(id)" class="jenkins-button jenkins-!-margin-4 jenkins-!-padding-0" style="display: inline; width:8em;" tooltip="Incomplete tests"> | ||
<p>${it.incompleteCount}<br> | ||
<l:icon class="symbol-alert-circle-outline plugin-ionicons-api icon-lg jenkins-!-warning-color"/> | ||
</br><br></br>Incomplete</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test browser uses different icons for incomplete and "not run" tests.
src/main/resources/com/mathworks/ci/TestResultsViewAction/index.jelly
Outdated
Show resolved
Hide resolved
src/main/resources/com/mathworks/ci/TestResultsViewAction/index.jelly
Outdated
Show resolved
Hide resolved
Test Name | ||
</th> | ||
<th class="pane-header" style="width: auto; min-width: 10em; text-align: right;"> | ||
Duration (seconds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(s) instead of (seconds)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to include + and - signs or drop-down arrows instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated (seconds) to (s)
Is it better to include + and - signs or drop-down arrows instead?
I didn't get this, could you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing major. Currently, it seems that you use the + and - buttons to expand and collapse sections in the report. I was wondering whether a drop-down arrow could do the same. In the test browser, we resort to the arrow for expansion: https://www.mathworks.com/help/matlab/matlab_prog/run-tests-using-test-browser.html#:~:text=You%20can%20also%20add%20tests%20to%20the%20test%20browser%20by%20clicking%20the%20drop%2Ddown%20arrow
src/main/resources/com/mathworks/ci/TestResultsViewAction/index.jelly
Outdated
Show resolved
Hide resolved
src/main/resources/com/mathworks/ci/TestResultsViewAction/index.jelly
Outdated
Show resolved
Hide resolved
pom.xml
Outdated
<artifactId>bom-2.164.x</artifactId> | ||
<version>4</version> | ||
<artifactId>bom-2.346.x</artifactId> | ||
<version>1763.v092b_8980a_f5e</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since We are updating this, could we consider updating it to the latest ? https://github.com/jenkinsci/bom/blob/master/README.md
|
||
public List<List<TestFile>> getTestResults() throws ParseException, InterruptedException, IOException { | ||
List<List<TestFile>> testResults = new ArrayList<>(); | ||
FilePath fl = new FilePath(new File(build.getRootDir().getAbsolutePath() + File.separator + MatlabBuilderConstants.TEST_RESULTS_VIEW_ARTIFACT + this.actionID + ".json")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work for a Windows server and Linux agent and vice-versa? I've sometimes run into trouble with using File.separator
in those cases.
I'm always a little nervous about working with file paths in the Jenkins plugin because it's really easy to get a path to a file on the agent that doesn't exist on the server and vice-versa. Something I mess up all the time haha.
This case looks good to me though as all of the action code should be running on the agent!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't get me started on file paths issue with Jenkins, I've devoted so much time to fixing those 😭
Haven't tested this combination yet but sure will test it before releasing.
This feature creates an in-platform view for MATLAB test results (whenever a default test runner or pre-made TestTask is used) if the user has a MATLAB Test license.
Key highlights:
Here's a few snapshots: