Update right click menu label based on test status #811
Conversation
brpowell
changed the title
| { | ||
| "command": "sfdx.force.test.view.showDefinition", | ||
| "when": | ||
| "view == sfdx.force.test.view && viewItem =~ /(apexTest|apexTestGroup)(_Pass|_Skip|\\b)/" |
There was a problem hiding this comment.
VSCode doesn't exactly have a way of dynamically updating the labels for the same command in a context menu. I created another command that runs the same method, but has the "Go to Definition" label. Then conditionally render one command or the other based on the where clause.
There was a problem hiding this comment.
- Yeah, there isn't a way to dynamically update the labels.
- Does this mean that you won't be able to see the "Go to Definition" menu if you are on a failing node?
- What about calling it
goToDefinition. I think you're really going to the definition more than showing it in-line.
| @@ -246,6 +246,9 @@ export abstract class TestNode extends vscode.TreeItem { | |||
| dark: DARK_ORANGE_BUTTON | |||
| }; | |||
| } | |||
|
|
|||
| const contextValue = this.contextValue.split('_'); | |||
| this.contextValue = `${contextValue}_${outcome}`; | |||
There was a problem hiding this comment.
Changed the name of this method to highlight it's doing more than updating an icon now. I'm appending the outcome of a test to the contextValue of the test node so i can access this in the commands' where clauses as defined in the package.json.
| @@ -34,11 +35,22 @@ export class ApexTestRunner { | |||
| } | |||
|
|
|||
| public async showErrorMessage(test: TestNode) { | |||
There was a problem hiding this comment.
When a test class is marked as failed, the "Display Error" option will still show up upon right clicking it. It seemed confusing to have Display Error bring you to the top of the class file, so I changed it to go to the first failed test case.
brpowell
requested review from
AnanyaJha,
vazexqi,
allileong,
JimSuplizio,
lcampos and
sfsholden
| { | ||
| "command": "sfdx.force.test.view.showDefinition", | ||
| "when": | ||
| "view == sfdx.force.test.view && viewItem =~ /(apexTest|apexTestGroup)(_Pass|_Skip|\\b)/" |
There was a problem hiding this comment.
- Yeah, there isn't a way to dynamically update the labels.
- Does this mean that you won't be able to see the "Go to Definition" menu if you are on a failing node?
- What about calling it
goToDefinition. I think you're really going to the definition more than showing it in-line.
| @@ -6,6 +6,7 @@ | |||
| "refresh_test_title": "Refresh Tests", | |||
| "run_single_test_title": "Run Single Test", | |||
| "show_error_title": "Display Error", | |||
| "show_definition_title": "Go to Definition", | |||
There was a problem hiding this comment.
"show_definition_title" -> "go_to_definition_title"?
There was a problem hiding this comment.
- Right, you'll only ever see "Go to Definition" or "Display Error". Do you think we should have both options for a failing node?
- Yeah I should be consistent, I'll update that.
Codecov Report
@@ Coverage Diff @@
## develop #811 +/- ##
==========================================
+ Coverage 72.76% 73.27% +0.5%
==========================================
Files 167 167
Lines 6790 6799 +9
Branches 1069 1073 +4
==========================================
+ Hits 4941 4982 +41
+ Misses 1582 1546 -36
- Partials 267 271 +4
Continue to review full report at Codecov.
|
brpowell
changed the title
| parseJSONStub.restore(); | ||
| }); | ||
|
|
||
| it('Go to definition if a test does not have an error message', () => { |
There was a problem hiding this comment.
Lets update the test titles to start with 'Should' and keep them consistent with the rest, e.g. Should go to definition if a test ....
| @@ -33,12 +34,23 @@ export class ApexTestRunner { | |||
| this.eventsEmitter.on('sfdx:update_selection', this.updateSelection); | |||
| } | |||
|
|
|||
| public async showErrorMessage(test: TestNode) { | |||
| public showErrorMessage(test: TestNode) { | |||
There was a problem hiding this comment.
Very cool to get rid of the async here.
…salesforcedx-vscode into bpowell/testRightClickLabel
…salesforcedx-vscode into bpowell/testRightClickLabel
Looking for some feedback on how I went about this. Also noticed there aren't any tests for the functionality of going to the error/definition, I'll work on that as well.
What does this PR do?
When right clicking on a test that hasn't run, passed, or is skipped, the menu option now says "Go to Definition" instead of "Display Error", the latter which is shown only when the test has failed.
What issues does this PR fix or reference?
W-5368845