Skip to content
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

feat: add code coverage and junit reporting #491

Merged
merged 34 commits into from
May 26, 2022
Merged

Conversation

peternhale
Copy link
Contributor

What does this PR do?

Add code coverage and junit reporting for mdapi deploy, deploy report, source deploy and source deploy report commands

What issues does this PR fix or reference?

@AW-11025210@

@peternhale peternhale requested review from mshanemc and mdonnalley May 20, 2022 13:45
src/deployCommand.ts Outdated Show resolved Hide resolved
src/deployCommand.ts Outdated Show resolved Hide resolved
src/deployCommand.ts Outdated Show resolved Hide resolved
@peternhale peternhale requested review from maggiben and mdonnalley May 20, 2022 18:13
peternhale and others added 8 commits May 23, 2022 05:56
Co-authored-by: Juliet Shackell <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
Co-authored-by: Juliet Shackell <[email protected]>
src/coverageUtils.ts Outdated Show resolved Hide resolved
import { SourceCommand } from './sourceCommand';
import { DeployData, Stash } from './stash';
import { transformCoverageToApexCoverage, transformDeployTestsResultsToTestResult } from './coverageUtils';
// TODO: this function needs to be moved to a shared location
import { toArray } from './formatters/resultFormatter';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose not to switch. SDR does not formally export that function. To me that means it is not meant to be consumed by others.

@@ -162,6 +181,85 @@ export abstract class DeployCommand extends SourceCommand {

return this.isAsync ? this.report(validatedDeployId) : this.poll(validatedDeployId);
}

protected createRequestedReports(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to name these kind of things maybeCreateRequestedReports so that when you're reading it from the command, it's obvious that it might no-op.

coverageReport.generateReports();
}

protected getCoverageFormattersOptions(formatters: string[] = []): CoverageReporterOptions {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there are only certain strings that would be valid, you could use the typings here. Then, increateRequestedReports, you can use this.getFlag<CoverageReportFormats[]>(this.flags.coverageformatters)

Suggested change
protected getCoverageFormattersOptions(formatters: string[] = []): CoverageReporterOptions {
protected getCoverageFormattersOptions(formatters: CoverageReportFormats[] = []): CoverageReporterOptions {

src/deployCommand.ts Outdated Show resolved Hide resolved
const junitResults = jUnitReporter.format(testResult);

const junitReportPath = path.join(this.outputDir, 'junit');
fs.mkdirSync(junitReportPath, { recursive: true });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this, and createCoverageReport I'd prefer that methods with fs ops be async and use the async fs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into switch to async

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this async would require a larger refactor. If that is to be done, I suggest a separate work item or wait until these commands appear in sf.

Co-authored-by: Shane McLaughlin <[email protected]>
@maggiben
Copy link
Contributor

Coverage report for dreamhouse-lwc:

=== Test Results Summary
Passing: 7
Failing: 0
Total: 7
Time: 0

Sample classes:

GeocodingService w/100% coverage:

image

<file name="GeocodingService" path="no-map/GeocodingService">
      <metrics statements="1" coveredstatements="1" conditionals="0" coveredconditionals="0" methods="0" coveredmethods="0"/>
      <line num="Infinity" count="1" type="stmt"/>
    </file>

PropertyController w/95% coverage:

image

<file name="PropertyController" path="no-map/PropertyController">
      <metrics statements="46" coveredstatements="44" conditionals="0" coveredconditionals="0" methods="0" coveredmethods="0"/>
      <line num="26" count="0" type="stmt"/>
      <line num="27" count="1" type="stmt"/>
      <line num="28" count="1" type="stmt"/>
      <line num="29" count="1" type="stmt"/>
      <line num="30" count="1" type="stmt"/>
      <line num="31" count="0" type="stmt"/>
      <line num="32" count="1" type="stmt"/>
      <line num="33" count="1" type="stmt"/>
      <line num="34" count="1" type="stmt"/>
      <line num="35" count="1" type="stmt"/>
      <line num="36" count="1" type="stmt"/>
      <line num="37" count="1" type="stmt"/>
      <line num="38" count="1" type="stmt"/>
      <line num="39" count="1" type="stmt"/>
      <line num="40" count="1" type="stmt"/>
      <line num="41" count="1" type="stmt"/>
      <line num="42" count="1" type="stmt"/>
      <line num="43" count="1" type="stmt"/>
      <line num="44" count="1" type="stmt"/>
      <line num="45" count="1" type="stmt"/>
      <line num="46" count="1" type="stmt"/>
      <line num="47" count="1" type="stmt"/>
      <line num="48" count="1" type="stmt"/>
      <line num="49" count="1" type="stmt"/>
      <line num="50" count="1" type="stmt"/>
      <line num="51" count="1" type="stmt"/>
      <line num="52" count="1" type="stmt"/>
      <line num="53" count="1" type="stmt"/>
      <line num="54" count="1" type="stmt"/>
      <line num="55" count="1" type="stmt"/>
      <line num="56" count="1" type="stmt"/>
      <line num="57" count="1" type="stmt"/>
      <line num="58" count="1" type="stmt"/>
      <line num="59" count="1" type="stmt"/>
      <line num="60" count="1" type="stmt"/>
      <line num="61" count="1" type="stmt"/>
      <line num="62" count="1" type="stmt"/>
      <line num="63" count="1" type="stmt"/>
      <line num="64" count="1" type="stmt"/>
      <line num="65" count="1" type="stmt"/>
      <line num="66" count="1" type="stmt"/>
      <line num="67" count="1" type="stmt"/>
      <line num="68" count="1" type="stmt"/>
      <line num="69" count="1" type="stmt"/>
      <line num="70" count="1" type="stmt"/>
      <line num="71" count="1" type="stmt"/>
    </file>

*(Note line 26 & 31 ternary is not covered)

@maggiben
Copy link
Contributor

Class with 0% coverage:
image

    <file name="myApexClass" path="no-map/myApexClass">
      <metrics statements="16" coveredstatements="8" conditionals="0" coveredconditionals="0" methods="0" coveredmethods="0"/>
      <line num="3" count="0" type="stmt"/>
      <line num="4" count="1" type="stmt"/>
      <line num="5" count="1" type="stmt"/>
      <line num="6" count="0" type="stmt"/>
      <line num="7" count="0" type="stmt"/>
      <line num="8" count="0" type="stmt"/>
      <line num="9" count="0" type="stmt"/>
      <line num="10" count="0" type="stmt"/>
      <line num="11" count="0" type="stmt"/>
      <line num="12" count="1" type="stmt"/>
      <line num="13" count="0" type="stmt"/>
      <line num="14" count="1" type="stmt"/>
      <line num="15" count="1" type="stmt"/>
      <line num="16" count="1" type="stmt"/>
      <line num="17" count="1" type="stmt"/>
      <line num="18" count="1" type="stmt"/>
    </file>

Copy link
Contributor

@jshackell-sfdc jshackell-sfdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my suggestions.

@maggiben
Copy link
Contributor

maggiben commented May 26, 2022

Testing Levels for source:deploy:

force:source:deploy with testlevel = NoTestRun produces no results
force:source:deploy with testlevel = RunSpecifiedTests produces results associated with theruntests passed to the command and writes coverage and junit results to resultsdir
force:source:deploy with testlevel = RunLocalTests writes coverage and junit results to resultsdir
force:source:deploy with testlevel = RunAllTestsInOrg produces test coverage and junit summary and results are written to the resultsdir

Asynchronous warning message for source:deploy & mdapi:deploy:

Warning: You requested an async deploy with coverage or junit. The reports will not be available until the deploy completes. is displayed

Testing Levels for mdapi:deploy:

force:mdapi:deploy with testlevel = NoTestRun produces no results
force:source:deploy with testlevel = RunSpecifiedTests fails to deploy (fails to meet the coverage expectations in the org) but this is unrelated to this change.
force:mdapi:deploy with testlevel = RunLocalTests writes coverage and junit results to resultsdir
force:mdapi:deploy with testlevel = RunAllTestsInOrg writes coverage and junit results to resultsdir

@maggiben maggiben merged commit 3e6ff90 into main May 26, 2022
@maggiben maggiben deleted the phale/deploy-coverage-junit branch May 26, 2022 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants