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

🌱 use ValidateTestReturn for Code-Review tests #3897

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

test cleanup, similar to #3872.
This is the first step before hooking up the Code-Review probes to the evaluation code

What is the current behavior?

we only checked the result score, which produces test failures without clear actionable feedback.

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

  • We use our test helper: ValidateTestReturn, which provides much more actionable feedback when tests fail
  • ctrl.Finish() is removed, as it's not needed after Go 1.14
  • simplified err checking logic
  • removed some unneeded lints
  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

NONE

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

By using ValidateTestReturn, we can get more actionable failure messages

Signed-off-by: Spencer Schrock <[email protected]>
Signed-off-by: Spencer Schrock <[email protected]>
@spencerschrock spencerschrock requested a review from a team as a code owner February 22, 2024 19:23
@spencerschrock spencerschrock requested review from raghavkaul and laurentsimon and removed request for a team February 22, 2024 19:23
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Merging #3897 (a41c158) into main (b972699) will decrease coverage by 4.84%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3897      +/-   ##
==========================================
- Coverage   75.50%   70.67%   -4.84%     
==========================================
  Files         231      231              
  Lines       15669    15669              
==========================================
- Hits        11831    11074     -757     
- Misses       3109     3917     +808     
+ Partials      729      678      -51     

@spencerschrock spencerschrock merged commit c7f6efe into ossf:main Feb 23, 2024
41 checks passed
@spencerschrock spencerschrock deleted the cleanup-code-review-test branch February 23, 2024 18:34
fhoeborn pushed a commit to fhoeborn/scorecard that referenced this pull request Apr 1, 2024
* check code review log messages, not just score

By using ValidateTestReturn, we can get more actionable failure messages

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

* simplify error checking

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

---------

Signed-off-by: Spencer Schrock <[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