Visual display of apex code coverage #1145

lcampos · 2019-03-13T00:54:15Z

What does this PR do?

It takes the info from the test run results that contain code coverage and highlights which lines those test results cover and which they don't.

What issues does this PR fix or reference?

#973

lcampos · 2019-03-13T00:55:55Z

packages/salesforcedx-vscode-apex/src/codecoverage/colorizer.ts

+  const codeCoverage = JSON.parse(testResultOutput) as CoverageTestResult;
+  if (codeCoverage.coverage === undefined) {
+    throw new Error(
+      `No code coverage information was found for test run ${testRunId}.`


Error showed when the test results do not include code coverage information.

What would we recommend that people do next? Set "salesforcedx-vscode-core.retrieve-test-code-coverage": true? Or did something else probably cause this error?

Yes, running tests without that setting on.

lcampos · 2019-03-13T00:56:51Z

packages/salesforcedx-vscode-apex/src/codecoverage/colorizer.ts

+function getTestRunId() {
+  const testRunIdFile = path.join(apexDirPath, 'test-run-id.txt');
+  if (!fs.existsSync(testRunIdFile)) {
+    throw new Error('No test run information was found for this project.');


Error when the project does not have any test run information, this would happen if they've never run tests via vscode/cli.

Can we add a next step? Something like, 'No test run information was found for this project. Set "salesforcedx-vscode-core.retrieve-test-code-coverage": true in your user or workspace settings, then run Apex tests from the Apex Tests sidebar or from within a test class file.'

lcampos · 2019-03-13T00:57:44Z

packages/salesforcedx-vscode-apex/src/codecoverage/colorizer.ts

+
+        if (!codeCovItem) {
+          throw new Error(
+            'No code coverage information was found for the current file.'


We'll show this when the test results do have code coverage information but not for the apex file currently in focus.

Can we add a next step? Something like, 'No code coverage information was found for this file. Set "salesforcedx-vscode-core.retrieve-test-code-coverage": true in your user or workspace settings. Then, run Apex tests that include methods in this file from the Apex Tests sidebar or using the Run Tests or Run All Tests code lens within the file.

lcampos · 2019-03-13T00:58:17Z

packages/salesforcedx-vscode-apex/src/codecoverage/decorations.ts

+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+
+import { window } from 'vscode';


These are the color definitions used (green & red).

lcampos · 2019-03-13T00:59:31Z

packages/salesforcedx-vscode-apex/src/index.ts

+  const statusBarToggle = new StatusBarToggle();
+  const colorizer = new CodeCoverage(statusBarToggle);
+  const colorizerCmd = vscode.commands.registerCommand(
+    'sfdx.force.apex.toggle.colorizer',


Tied everything under one command so that it's easy to trigger the feature using keyboard shortcuts.

lcampos · 2019-03-13T01:06:15Z

...ystem-tests/assets/sfdx-simple/.sfdx/tools/testresults/apex/test-result-707xx00000000BG.json

@@ -0,0 +1,105 @@
+{


Had to add these files under testresults in order to run the tests. Let me know if you have a better idea for generating these for testing purposes.

lcampos · 2019-03-13T01:07:08Z

packages/salesforcedx-vscode-apex/src/codecoverage/statusBarToggle.ts

+    'sfdx.force.apex.toggle.colorizer';
+  private static readonly showIcon = '$(tasklist)';
+  private static readonly hideIcon = '$(three-bars)';
+  private static readonly toolTip = 'Apex Code Coverage highlighter';


Text showed when you hover the status bar icon.

Other nearby hover text seems to use imperative verbs and title-case capitalization. How about this? 'Highlight Apex Code Coverage'

codecov · 2019-03-13T01:22:52Z

Codecov Report

Merging #1145 into develop will increase coverage by 0.1%.
The diff coverage is 79.59%.

@@            Coverage Diff            @@
##           develop   #1145     +/-   ##
=========================================
+ Coverage    72.59%   72.7%   +0.1%     
=========================================
  Files          189     193      +4     
  Lines         7124    7222     +98     
  Branches       731     746     +15     
=========================================
+ Hits          5172    5251     +79     
- Misses        1803    1814     +11     
- Partials       149     157      +8

Impacted Files	Coverage Δ
...salesforcedx-vscode-apex/src/codecoverage/index.ts	`0% <0%> (ø)`
...orcedx-vscode-apex/src/codecoverage/decorations.ts	`100% <100%> (ø)`
...sforcedx-vscode-apex/src/codecoverage/colorizer.ts	`76.05% <76.05%> (ø)`
...dx-vscode-apex/src/codecoverage/statusBarToggle.ts	`95% <95%> (ø)`
...forcedx-vscode-core/src/channels/channelService.ts	`85.71% <0%> (+2.38%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdedef7...e808340. Read the comment docs.

ruthemmanuelle · 2019-03-13T15:39:02Z

Glad to see this, @lcampos . I added some comments on the messages. Also, shouldn't the messages be externalized into messages files?

praksb · 2019-03-13T21:20:35Z

packages/salesforcedx-vscode-apex/src/index.ts

@@ -130,6 +139,7 @@ function registerCommands(
    forceApexTestMethodRunCodeAction
  );
  return vscode.Disposable.from(
+    colorizerCmd,


should this be forceApexToggleColorizerCmd to match the rest?

praksb · 2019-03-13T21:21:47Z

packages/salesforcedx-vscode-apex/src/index.ts

+  // Colorize code coverage
+  const statusBarToggle = new StatusBarToggle();
+  const colorizer = new CodeCoverage(statusBarToggle);
+  const colorizerCmd = vscode.commands.registerCommand(


As per the pattern i am seeing for other commands, i think this could be renamed to forceApexToggleColorizerCmd.

praksb · 2019-03-13T21:29:15Z

@lcampos what happens if we try this feature on a stale run result? Let's say that the apex class has since been edited and a bunch of methods have been removed. Does attempting to map nonexistent lines fail gracefully?

lcampos · 2019-03-15T17:03:33Z

@praksb it does fail gracefully. If the code just shifted but the lines that will be colored are still in the range of the document, then we will just color whatever code, comment, empty space available. If the document's length has shortened, then it will just display a warning about it if the lines that need to be colored are not part of the range of the document.
I'll add a message to let the customer know they need to refresh their code coverage run but it will only be displayed in the last case described above since we do not do further validations to the code in the file or code coverage run. I wanted to keep the feature as simple as possible in the first version and then see if we needed to add more validations via apex lsp, etc.

lcampos · 2019-03-15T17:05:57Z

@ruthemmanuelle thanks for the review, yes we will have these messages on the extension's i18n.ts file. Since I didn't know how many different labels we were gonna need I just kept it on the code.

AnanyaJha

The command renames Prakash mentioned seem like a good idea, otherwise the changes look cool!

lcampos · 2019-03-15T18:41:29Z

packages/salesforcedx-vscode-apex/src/messages/i18n.ts

@@ -40,5 +40,14 @@ export const messages = {

  client_name: 'Apex Language Server',
  cannot_determine_workspace:
-    'Unable to determine workspace folders for workspace'
+    'Unable to determine workspace folders for workspace',


@ruthemmanuelle here are the new error messages, let me know if any changes are needed.

…dle some common scenarios where coverage info is not available

lcampos commented Mar 13, 2019

View reviewed changes

lcampos requested review from JimSuplizio, AnanyaJha, allileong, sfsholden, praksb, brpowell and ruthemmanuelle March 13, 2019 01:07

praksb reviewed Mar 13, 2019

View reviewed changes

AnanyaJha approved these changes Mar 15, 2019

View reviewed changes

lcampos force-pushed the lcampos/colorizer branch from 8981e06 to d198fe0 Compare March 15, 2019 18:40

lcampos commented Mar 15, 2019

View reviewed changes

praksb approved these changes Mar 15, 2019

View reviewed changes

Luis Campos added 5 commits March 15, 2019 18:38

Working draft for apex code coverage highlighting

c621003

Add code cov coloring removal, clean up code and restructure it

f3aa6a0

Add error handling and messaging when code coverage data is not present

a3d642e

Move to have only one command to toggle rendering the decorators. Han…

e5cec73

…dle some common scenarios where coverage info is not available

Add tests to code coverage colorizer

fd2682c

Luis Campos added 5 commits March 15, 2019 18:38

Add missing copyright header

483d8de

Include test code coverage files

c9607f5

Updates based on review feedback

cdf96be

Update test for autobuilds

96261a0

Fix windows file path parsing issue

e808340

lcampos force-pushed the lcampos/colorizer branch from d198fe0 to e808340 Compare March 16, 2019 01:39

lcampos merged commit be06b64 into develop Mar 16, 2019

lcampos deleted the lcampos/colorizer branch March 16, 2019 02:11

ruthemmanuelle mentioned this pull request Mar 18, 2019

Edit code coverage messages #1159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Visual display of apex code coverage #1145

Visual display of apex code coverage #1145

lcampos commented Mar 13, 2019 •

edited by ntotten

Loading

lcampos Mar 13, 2019

ruthemmanuelle Mar 13, 2019

lcampos Mar 15, 2019

lcampos Mar 13, 2019

ruthemmanuelle Mar 13, 2019

lcampos Mar 13, 2019

ruthemmanuelle Mar 13, 2019

lcampos Mar 13, 2019

lcampos Mar 13, 2019

lcampos Mar 13, 2019

lcampos Mar 13, 2019

ruthemmanuelle Mar 13, 2019

codecov bot commented Mar 13, 2019 •

edited

Loading

ruthemmanuelle commented Mar 13, 2019

praksb Mar 13, 2019

praksb Mar 13, 2019

praksb commented Mar 13, 2019

lcampos commented Mar 15, 2019

lcampos commented Mar 15, 2019

AnanyaJha left a comment

lcampos Mar 15, 2019

Visual display of apex code coverage #1145

Visual display of apex code coverage #1145

Conversation

lcampos commented Mar 13, 2019 • edited by ntotten Loading

What does this PR do?

What issues does this PR fix or reference?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 13, 2019 • edited Loading

Codecov Report

ruthemmanuelle commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

praksb commented Mar 13, 2019

lcampos commented Mar 15, 2019

lcampos commented Mar 15, 2019

AnanyaJha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcampos commented Mar 13, 2019 •

edited by ntotten

Loading

codecov bot commented Mar 13, 2019 •

edited

Loading