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

✨ scdiff: normalize scorecard results #3294

Merged
merged 6 commits into from
Jul 31, 2023

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

feature

What is the current behavior?

scorecard results were printed verbatim by the scdiff tool

What is the new behavior (if this is a feature change)?**

certain fields, such as dates or commit shas, are stripped before printing.
these fields are informational, but don't provide value in detecting a change in scorecard behavior. instead they interfere with the comparison

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Related to #2462, 2/n

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

NONE

@spencerschrock spencerschrock temporarily deployed to gitlab July 21, 2023 20:20 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test July 21, 2023 20:21 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #3294 (4c87f13) into main (f8285ff) will decrease coverage by 6.63%.
The diff coverage is 95.00%.

❗ Current head 4c87f13 differs from pull request most recent head 0a863f0. Consider uploading reports for the commit 0a863f0 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3294      +/-   ##
==========================================
- Coverage   72.83%   66.21%   -6.63%     
==========================================
  Files         173      173              
  Lines       12700    12703       +3     
==========================================
- Hits         9250     8411     -839     
- Misses       2944     3824     +880     
+ Partials      506      468      -38     

Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

Thanks.

Can you please add tests for the missing lines?

@spencerschrock
Copy link
Member Author

spencerschrock commented Jul 26, 2023

Thanks.

Can you please add tests for the missing lines?

It's a cobra command, which runs scorecard on a repo URI. This would be difficult to do as written. The diff coverage in this PR is already 80%

Edit: Misread your message slightly. Can try to get the error lines covered in format.go, but the line in generate.go might be out of scope

@spencerschrock spencerschrock temporarily deployed to gitlab July 26, 2023 17:09 — with GitHub Actions Inactive
@spencerschrock spencerschrock temporarily deployed to integration-test July 26, 2023 17:10 — with GitHub Actions Inactive
@spencerschrock
Copy link
Member Author

Covered an additional line in format.go. I don't think it's possible for checks.Read() to error, as it's unmarshalling an embeded file we know unmarshalls fine.

Please take another look @naveensrinivasan

Copy link
Member

@naveensrinivasan naveensrinivasan left a comment

Choose a reason for hiding this comment

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

Thanks!

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
@naveensrinivasan naveensrinivasan force-pushed the golden/generate-format branch from 4c87f13 to 0a863f0 Compare July 31, 2023 20:45
@naveensrinivasan naveensrinivasan enabled auto-merge (squash) July 31, 2023 20:45
@naveensrinivasan naveensrinivasan temporarily deployed to gitlab July 31, 2023 20:45 — with GitHub Actions Inactive
@naveensrinivasan naveensrinivasan temporarily deployed to integration-test July 31, 2023 20:45 — with GitHub Actions Inactive
@naveensrinivasan naveensrinivasan merged commit b829961 into ossf:main Jul 31, 2023
@spencerschrock spencerschrock deleted the golden/generate-format branch August 7, 2023 19:27
ashearin pushed a commit to kgangerlm/scorecard-gitlab that referenced this pull request Nov 13, 2023
* Normalize results.

Signed-off-by: Spencer Schrock <[email protected]>

* add tests for standardized output format

Signed-off-by: Spencer Schrock <[email protected]>

* move comment to proper place.

Signed-off-by: Spencer Schrock <[email protected]>

* add comment about pretty print.

Signed-off-by: Spencer Schrock <[email protected]>

* format test can be parallel.

Signed-off-by: Spencer Schrock <[email protected]>

* Add normalize nil test.

Signed-off-by: Spencer Schrock <[email protected]>

---------

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Allen Shearin <[email protected]>
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.

2 participants