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

Human formatted Apex test report shouldn't display individual PASSING test methods #243

Closed
daveespo opened this issue Sep 7, 2021 · 11 comments
Labels
enhancement New feature or request

Comments

@daveespo
Copy link

daveespo commented Sep 7, 2021

Some of our projects have upwards of 2000 test methods. When we run force:apex:test:run -w 50 -r human in our CI/CI scripts and there is a single failed test method, we see 1999 lines of "Pass" and we have to painfully try to hunt down the single test method with the Fail status.

In a prior version of the CLI, it would only report test failures which meant that for a 100% successful test run, it would only print the summary block with the outcome of the test run -- there's no need to print 2000 lines of noise in the log file telling me that each test method was successful

Something changed at some point within the past few months (basically at the point where passing -r human was required in order to see the detail of the test failures)

Solution 1 - silence test methods that Pass by default

Pros: This is the way it should be and the way just about every other test reporter I've ever seen in my life has worked. Users that want all Passing test methods to be enumerated can use the --verbose existing command line argument to enable.

Cons: This will change existing behavior

Solution 2 - Add a command line argument (eg. --not-so-verbose) to silence Pass test methods from the output

Pros: Preserves existing behavior

Cons: Requires updating existing CI/CD scripts to pass this new flag. Also, documenting and naming the flag to make it somehow compatible with the --verbose documentation could be challenging.

Additional context
Also, it should be noted that in the Summary table, the "Pass rate" is a whole number (presumably using half-up rounding logic) which results in incongruous outcome:

=== Test Summary
NAME                 VALUE
───────────────────  ──────────────────────────
Outcome              Failed
Tests Ran            870
Pass Rate            100%
Fail Rate            0%
Skip Rate            0%
Test Run Id          7071F0000279qGm
Test Execution Time  280082 ms
Org Id               00D1F000000aI3mUAE
Username             [email protected]

Which is it? A failure or 100% pass? What actually happened is one test method failed but that still resulted in the "99.9%" getting rounded up to 100%

Version that I'm running

@oclif/plugin-autocomplete 0.3.0 (core)
@oclif/plugin-commands 1.3.0 (core)
@oclif/plugin-help 3.2.2 (core)
@oclif/plugin-not-found 1.2.4 (core)
@oclif/plugin-plugins 1.10.1 (core)
@oclif/plugin-update 1.4.0-3 (core)
@oclif/plugin-warn-if-update-available 1.7.0 (core)
@oclif/plugin-which 1.0.3 (core)
@salesforce/sfdx-plugin-lwc-test 0.1.7 (core)
@salesforce/sfdx-trust 3.6.0 (core)
alias 1.1.10 (core)
apex 0.2.8 (core)
auth 1.7.1 (core)
config 1.2.24 (core)
custom-metadata 1.0.12 (core)
data 0.6.1 (core)
etcopydata 0.6.4-Beta (beta)
generator 1.1.7 (core)
limits 1.2.1 (core)
org 1.7.0 (core)
salesforce-alm 52.3.1 (core)
schema 1.0.8 (core)
sfdx-cli 7.116.2 (core)
shane-sfdx-plugins 4.43.0
├─ @mshanemc/plugin-streaming 1.1.7
└─ @mshanemc/sfdx-sosl 1.1.0
source 1.0.12 (core)
telemetry 1.2.3 (core)
templates 52.1.0 (core)
user 1.4.0 (core)
@daveespo
Copy link
Author

@mshanemc -- @k-capehart was so kind as to do the thing with PR#377 -- what is the process for getting this change considered?

It could be viewed as a breaking change .. but it's been repeated a million times over that human readable output should not be relied on to remain consistent .. so I think this would qualify as a permitted change.

@k-capehart
Copy link
Contributor

k-capehart commented Jun 12, 2024

@mshanemc -- @k-capehart was so kind as to do the thing with PR#377 -- what is the process for getting this change considered?

It could be viewed as a breaking change .. but it's been repeated a million times over that human readable output should not be relied on to remain consistent .. so I think this would qualify as a permitted change.

This particular change in the node library is not breaking because I've defaulted the verbose value to true.

However, if we do add a --verbose flag to the cli command then that would technically change behavior. I agree with you that it shouldn't be relied on to be consistent. Regardless, it might be worth opening a new issue in the CLI repository specifically for that part of the discussion, since it'll be a new PR and specific to that team. Its just dependent on this being merged first.
https://github.com/forcedotcom/cli/issues

@VivekMChawla
Copy link

@k-capehart and @daveespo - Improving Apex test results is something I'd like to get onto our near-term roadmap.

Some of the suggestions I've seen include adding a --concise flag to produce output similar to what this PR suggests.

While our stated policy is that we reserve the right to change human-readable output at any time, the importance of Apex test results in automation use cases makes me want to tread carefully.

All the suggestions to improve Apex test results that I've seen would result in significant changes to human-readable output, and would be best delivered with matching changes to JSON output. So, if we're going to take a "breaking changes" hit, I'd like to solve as much as we can in the process.

To that end, I plan to start a GitHub discussion to consolidate existing feature requests and solicit new ideas. The CLI team will turn that into a design proposal for final feedback. Then, we'll build the changes and put the new behavior behind an environment variable for a period of time while the old behavior is deprecated.

FYI: @mshanemc

@k-capehart
Copy link
Contributor

@VivekMChawla Thanks for the reply. I appreciate the transparency as always. I'll keep an eye out for that discussion post and for ways to help so we can get this out sooner rather than later.

@k-capehart
Copy link
Contributor

@daveespo Just updating you. This is just pending the following PR: salesforcecli/plugin-apex#504

Once its approved then this should be good to go!

@dschach
Copy link

dschach commented Aug 1, 2024

Linking to forcedotcom/cli#2872 for reference

@daveespo
Copy link
Author

daveespo commented Aug 5, 2024

Yep, I've been watching .. fingers-crossed on quick approval and merge ;-)

@k-capehart
Copy link
Contributor

salesforcecli/plugin-apex#553 was merged to main, which contains the commits from my PR. Should be good to go in the next week or two for the next release! 🚀

@avesolovksyy
Copy link

@k-capehart
How about test results that are displayed as a part of sf force source deploy --checkonly --testlevel ... command? Seems like there is similar problem with output there and having --concise would be beneficial.

I haven't used yet sf project deploy start that is suggested as a replacement of to be deprecated deploy command above - hope that --concise flag there serves the same purpose, i.e. to display only failures right at the bottom of the screen.

@k-capehart
Copy link
Contributor

Hey @avesolovksyy , I completely agree. I would recommend opening an Issue in forcedotcom/cli to request this feature be added to project deploy start.

@jshackell-sfdc
Copy link

This issue has been fixed in release 2.56.7 (August 28, 2024).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants