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

✨ Consider 'src/test' test directories #2706

Merged

Conversation

raboof
Copy link
Contributor

@raboof raboof commented Mar 1, 2023

What kind of change does this PR introduce?

Enhancement: improve the quality of the default scoring

What is the current behavior?

Files in testdata directories are ignored by default, but files in src/test directories are not.

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

src/test is also recognized as a test directory

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

Which issue(s) this PR fixes

NONE

Special notes for your reviewer

This directory is found in projects that follow the Maven Standard Directory Layout, which is fairly widely used, not only in Maven but also in adjacent ecosystems like Gradle and clojure.

Longer-term this would become part of the default policy, see discussion at #1408 (comment) etc.

Does this PR introduce a user-facing change?

For more information on release notes see: https://git.k8s.io/release/cmd/release-notes/README.md

`src/test` directories are now considered test directories by default.

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.

Thank you!

@naveensrinivasan naveensrinivasan force-pushed the ignore-maven-project-layout-test-paths branch from ab32af0 to 165bcd0 Compare March 1, 2023 16:24
@naveensrinivasan naveensrinivasan enabled auto-merge (squash) March 1, 2023 16:24
@naveensrinivasan naveensrinivasan temporarily deployed to integration-test March 1, 2023 16:24 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #2706 (c2187a0) into main (846fb19) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2706      +/-   ##
==========================================
+ Coverage   41.31%   41.33%   +0.01%     
==========================================
  Files         123      123              
  Lines       10039    10041       +2     
==========================================
+ Hits         4148     4150       +2     
  Misses       5595     5595              
  Partials      296      296              

auto-merge was automatically disabled March 1, 2023 16:44

Head branch was pushed to by a user without write access

@raboof raboof force-pushed the ignore-maven-project-layout-test-paths branch from 165bcd0 to 0ee89ea Compare March 1, 2023 16:44
@raboof raboof force-pushed the ignore-maven-project-layout-test-paths branch from 0ee89ea to a41192f Compare March 2, 2023 08:34
@raboof
Copy link
Contributor Author

raboof commented Mar 2, 2023

(the e2e/sast_test.go failure seems to be a quota thing, but the e2e/fuzzing_test.go failure might be caused by this change - will dig in later)

@spencerschrock
Copy link
Contributor

(the e2e/sast_test.go failure seems to be a quota thing, but the e2e/fuzzing_test.go failure might be caused by this change - will dig in later)

can you rebase on main, or merge in the updates? As far as I know, the failing e2e test is due to a fuzzing e2e test which has been temporarily skipped since your branch was created

@raboof raboof force-pushed the ignore-maven-project-layout-test-paths branch from a41192f to a9363c9 Compare March 2, 2023 22:49
@raboof raboof temporarily deployed to integration-test March 2, 2023 22:50 — with GitHub Actions Inactive
The Maven 'Standard Directory Layout' [0] is fairly widely used, not only
in Maven but also in adjacent ecosystems like Gradle and clojure.

Longer-term this would become part of the default policy, see discussion
at ossf#1408 (comment) etc.

[0]: https://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html

Signed-off-by: Arnout Engelen <arnout@bzzt.net>
@raboof raboof force-pushed the ignore-maven-project-layout-test-paths branch from a9363c9 to c2187a0 Compare March 3, 2023 08:26
@raboof
Copy link
Contributor Author

raboof commented Mar 3, 2023

can you rebase on main, or merge in the updates? As far as I know, the failing e2e test is due to a fuzzing e2e test which has been temporarily skipped since your branch was created

Ah, thanks, that indeed seems to have done the trick. The unit tests fail because of an intermittent error:

time="2023-03-02T22:53:41Z" level=error msg="GitHub token env var is not set. Please read https://github.com/ossf/scorecard#authentication" error="an error occurred while getting GitHub credentials"
time="2023-03-02T22:53:42Z" level=info msg="Rate limit exceeded. Waiting 28m1.981501491s to retry..."

Triggered a new build for that by force-pushing without further changes.

@raboof raboof temporarily deployed to integration-test March 3, 2023 08:27 — with GitHub Actions Inactive
@naveensrinivasan naveensrinivasan merged commit 36faeac into ossf:main Mar 3, 2023
Shofiya2003 pushed a commit to Shofiya2003/scorecard that referenced this pull request Mar 10, 2023
The Maven 'Standard Directory Layout' [0] is fairly widely used, not only
in Maven but also in adjacent ecosystems like Gradle and clojure.

Longer-term this would become part of the default policy, see discussion
at ossf#1408 (comment) etc.

[0]: https://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html

Signed-off-by: Arnout Engelen <arnout@bzzt.net>
Signed-off-by: Shofiya2003 <shofiyabootwala@gmail.com>
Shofiya2003 pushed a commit to Shofiya2003/scorecard that referenced this pull request Mar 10, 2023
The Maven 'Standard Directory Layout' [0] is fairly widely used, not only
in Maven but also in adjacent ecosystems like Gradle and clojure.

Longer-term this would become part of the default policy, see discussion
at ossf#1408 (comment) etc.

[0]: https://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html

Signed-off-by: Arnout Engelen <arnout@bzzt.net>
Signed-off-by: Shofiya2003 <shofiyabootwala@gmail.com>
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.

None yet

3 participants