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

Check that there are "enough" tests and documentation #492

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
fad9538
Add a registry Guideline to enforce thresholds against statistics gle…
MarkNahabedian Jan 16, 2023
ec8f556
Unit test for linecounts_meet_thresholds.
MarkNahabedian Jan 17, 2023
11026cd
Record all Guidelines in ALL_GUIDELINES.
MarkNahabedian Jan 19, 2023
881dfba
Say which fields or keyargs are missing.
MarkNahabedian Jan 20, 2023
ebf74b8
Hopefully better warning message.
MarkNahabedian Jan 20, 2023
a6a07ac
Remove no longer applicable 'env_threshold' testset.
MarkNahabedian Jan 23, 2023
94f7a69
Further work on new Guideline guideline_linecounts_meet_thresholds to
MarkNahabedian Jan 24, 2023
dd04259
Merge branch 'master' into master
MarkNahabedian Jan 24, 2023
eee7c51
Oops, I need to be more carefful commenting out loggins.
MarkNahabedian Jan 24, 2023
d0b9996
Merge branch 'master' of https://github.com/MarkNahabedian/RegistryCI.jl
MarkNahabedian Jan 24, 2023
37bf9b6
PackageAnalyzer constrains Julia version to 1.6 or greater.
MarkNahabedian Jan 25, 2023
1b9e848
Update src/AutoMerge/guidelines.jl
MarkNahabedian Jan 26, 2023
75cbbf3
Update src/AutoMerge/guidelines.jl
MarkNahabedian Jan 26, 2023
48eb348
Revert 37bf9b6aef186c652f054db652d1e019807ad7c7 since the problem it …
MarkNahabedian Jan 26, 2023
cc90730
Merge branch 'master' of https://github.com/MarkNahabedian/RegistryCI.jl
MarkNahabedian Jan 26, 2023
6484879
Compat entry for PackageAnalyzer.
MarkNahabedian Jan 26, 2023
f36a8e0
Set defaults parameter values for guideline_linecounts_meet_threshold…
MarkNahabedian Jan 26, 2023
4ed3e65
Remove env_threshold_count which is no longer used.
MarkNahabedian Jan 26, 2023
edcf26d
meets_threshold now asserts bounds on thresholds.
MarkNahabedian Jan 26, 2023
f8b1275
Update Project.toml
ericphanson Jan 26, 2023
8c1dd57
Oops. Meant to remove the linecount thresholds from GitHubAutoMergeD…
MarkNahabedian Jan 26, 2023
507a16c
Version gate for guideline_linecounts_meet_thresholds because Package…
MarkNahabedian Jan 26, 2023
92fa52f
Merge branch 'master' of https://github.com/MarkNahabedian/RegistryCI.jl
MarkNahabedian Jan 26, 2023
d1e04e6
Add a separate set of parameters for ratio thresholds.
MarkNahabedian Feb 2, 2023
fc1d378
Can't test guideline_linecounts_meet_thresholds on Julia versions old…
MarkNahabedian Feb 2, 2023
04ef6f3
Update Project.toml
MarkNahabedian Apr 11, 2023
8155a87
Merge branch 'master' into master
DilumAluthge Apr 13, 2023
009fae8
Update src/AutoMerge/guidelines.jl documentation of parameters for gu…
MarkNahabedian Apr 18, 2023
c8ea113
Update guideline parameter count.
MarkNahabedian Apr 18, 2023
5d45493
remove using Logging.
MarkNahabedian Apr 18, 2023
5f2d565
Fix typo.
MarkNahabedian Apr 18, 2023
f14b2a6
Merge branch 'master' of https://github.com/MarkNahabedian/RegistryCI.jl
MarkNahabedian Apr 18, 2023
58eb4be
Attempt to accomodate those projects which shoose to put all of their…
MarkNahabedian Apr 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ HTTP = "cd3eb016-35fb-5094-929b-558a96fad6f3"
JSON = "682c06a0-de6a-54ab-a142-c8b1cf79cde6"
LibGit2 = "76f85450-5226-5b5a-8eaa-529ad045b433"
LicenseCheck = "726dbf0d-6eb6-41af-b36c-cd770e0f00cc"
Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
PackageAnalyzer = "e713c705-17e4-4cec-abe0-95bf5bf3e10c"
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
Printf = "de0858da-6303-5e67-8744-51eddeeeb8d7"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
Expand Down
119 changes: 98 additions & 21 deletions src/AutoMerge/guidelines.jl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using HTTP: HTTP
import PackageAnalyzer

const guideline_registry_consistency_tests_pass = Guideline(;
info="Registy consistency tests",
Expand All @@ -25,7 +26,8 @@ const guideline_compat_for_julia = Guideline(;
"There is an upper-bounded `[compat]` entry for `julia` that ",
"only includes a finite number of breaking releases of Julia.",
),
check=data -> meets_compat_for_julia(data.registry_head, data.pkg, data.version),
check=(data, guideline_parameters) ->
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved
meets_compat_for_julia(data.registry_head, data.pkg, data.version),
)

function meets_compat_for_julia(working_directory::AbstractString, pkg, version)
Expand Down Expand Up @@ -73,7 +75,8 @@ const guideline_compat_for_all_deps = Guideline(;
"are upper-bounded and only include a finite number of breaking releases. ",
"For more information, please see the \"Upper-bounded `[compat]` entries\" subsection under \"Additional information\" below.",
),
check=data -> meets_compat_for_all_deps(data.registry_head, data.pkg, data.version),
check=(data, guideline_parameters) ->
meets_compat_for_all_deps(data.registry_head, data.pkg, data.version),
)

function meets_compat_for_all_deps(working_directory::AbstractString, pkg, version)
Expand Down Expand Up @@ -150,12 +153,13 @@ end

const guideline_patch_release_does_not_narrow_julia_compat = Guideline(;
info="If it is a patch release on a post-1.0 package, then it does not narrow the `[compat]` range for `julia`.",
check=data -> meets_patch_release_does_not_narrow_julia_compat(
data.pkg,
data.version;
registry_head=data.registry_head,
registry_master=data.registry_master,
),
check=(data, guideline_parameters) ->
meets_patch_release_does_not_narrow_julia_compat(
data.pkg,
data.version;
registry_head=data.registry_head,
registry_master=data.registry_master,
),
)

function meets_patch_release_does_not_narrow_julia_compat(
Expand Down Expand Up @@ -200,7 +204,7 @@ const _AUTOMERGE_NEW_PACKAGE_MINIMUM_NAME_LENGTH = 5
const guideline_name_length = Guideline(;
info="Name not too short",
docs="The name is at least $(_AUTOMERGE_NEW_PACKAGE_MINIMUM_NAME_LENGTH) characters long.",
check=data -> meets_name_length(data.pkg),
check=(data, guideline_parameters) -> meets_name_length(data.pkg),
)

function meets_name_length(pkg)
Expand All @@ -215,7 +219,7 @@ end

const guideline_name_ascii = Guideline(;
info="Name is composed of ASCII characters only.",
check=data -> meets_name_ascii(data.pkg),
check=(data, guideline_parameters) -> meets_name_ascii(data.pkg),
)

function meets_name_ascii(pkg)
Expand All @@ -228,7 +232,7 @@ end

const guideline_julia_name_check = Guideline(;
info="Name does not include \"julia\" or start with \"Ju\".",
check=data -> meets_julia_name_check(data.pkg),
check=(data, guideline_parameters) -> meets_julia_name_check(data.pkg),
)

function meets_julia_name_check(pkg)
Expand Down Expand Up @@ -263,7 +267,8 @@ To prevent confusion between similarly named packages, the names of new packages
between the package name and any existing package must exceeds a certain
a hand-chosen threshold (currently 2.5).
""",
check=data -> meets_distance_check(data.pkg, data.registry_master),
check=(data, guideline_parameters) ->
meets_distance_check(data.pkg, data.registry_master),
)

function meets_distance_check(
Expand Down Expand Up @@ -377,7 +382,8 @@ const guideline_normal_capitalization = Guideline(;
"contain only ASCII alphanumeric characters, ",
"and contain at least one lowercase letter.",
),
check=data -> meets_normal_capitalization(data.pkg),
check=(data, guideline_parameters) ->
meets_normal_capitalization(data.pkg),
)

function meets_normal_capitalization(pkg)
Expand All @@ -392,7 +398,8 @@ end

const guideline_repo_url_requirement = Guideline(;
info="Repo URL ends with `/PackageName.jl.git`.",
check=data -> meets_repo_url_requirement(data.pkg; registry_head=data.registry_head),
check=(data, guideline_parameters) ->
meets_repo_url_requirement(data.pkg; registry_head=data.registry_head),
)

function meets_repo_url_requirement(pkg::String; registry_head::String)
Expand Down Expand Up @@ -446,7 +453,7 @@ const guideline_sequential_version_number = Guideline(;
"Invalid new versions include `1.0.2` (skips `1.0.1`), ",
"`1.3.0` (skips `1.2.0`), `3.0.0` (skips `2.0.0`) etc.",
),
check=data -> meets_sequential_version_number(
check=(data, guideline_parameters) -> meets_sequential_version_number(
data.pkg,
data.version;
registry_head=data.registry_head,
Expand Down Expand Up @@ -488,7 +495,8 @@ end

const guideline_standard_initial_version_number = Guideline(;
info="Standard initial version number. Must be one of: `0.0.1`, `0.1.0`, `1.0.0`, or `X.0.0`.",
check=data -> meets_standard_initial_version_number(data.version),
check=(data, guideline_parameters) ->
meets_standard_initial_version_number(data.version),
)

function meets_standard_initial_version_number(version)
Expand Down Expand Up @@ -540,7 +548,7 @@ end

const guideline_code_can_be_downloaded = Guideline(;
info="Code can be downloaded.",
check=data -> meets_code_can_be_downloaded(
check=(data, guideline_parameters) -> meets_code_can_be_downloaded(
data.registry_head,
data.pkg,
data.version,
Expand Down Expand Up @@ -609,7 +617,7 @@ end

const guideline_src_names_OK = Guideline(;
info="`src` files and directories names are OK",
check=data -> meets_src_names_ok(data.pkg_code_path),
check=(data, guideline_parameters) -> meets_src_names_ok(data.pkg_code_path),
)

function meets_code_can_be_downloaded(registry_head, pkg, version, pr; pkg_code_path)
Expand Down Expand Up @@ -662,7 +670,7 @@ is_valid_url(str::AbstractString) = !isempty(HTTP.URI(str).scheme) && isvalid(HT
const guideline_version_can_be_pkg_added = Guideline(;
info="Version can be `Pkg.add`ed",
docs="Package installation: The package should be installable (`Pkg.add(\"PackageName\")`).",
check=data -> meets_version_can_be_pkg_added(
check=(data, guideline_parameters) -> meets_version_can_be_pkg_added(
data.registry_head,
data.pkg,
data.version;
Expand Down Expand Up @@ -731,7 +739,8 @@ const guideline_version_has_osi_license = Guideline(;
"This check is required for the General registry. ",
"For other registries, registry maintainers have the option to disable this check.",
),
check=data -> meets_version_has_osi_license(data.pkg; pkg_code_path=data.pkg_code_path),
check=(data, guideline_parameters) ->
meets_version_has_osi_license(data.pkg; pkg_code_path=data.pkg_code_path),
)

function meets_version_has_osi_license(pkg::String; pkg_code_path)
Expand Down Expand Up @@ -793,7 +802,7 @@ end
const guideline_version_can_be_imported = Guideline(;
info="Version can be `import`ed",
docs="Package loading: The package should be loadable (`import PackageName`).",
check=data -> meets_version_can_be_imported(
check=(data, guideline_parameters) -> meets_version_can_be_imported(
data.registry_head,
data.pkg,
data.version;
Expand Down Expand Up @@ -857,6 +866,65 @@ function meets_version_can_be_imported(
end
end

const guideline_linecounts_meet_thresholds = Guideline(;
info="Test that lines of source, tests and documentation meet specified thresholds",
docs="""Make sure that various line counts meet or exceed specified thresholds.
Thresholds are controlled by these fields of GitHubAutoMergeData:
* src_min_lines: Minimum number of lines of source code
* readme_min_lines: % Minimum number of lines in the README file
* test_min_lines: % Minimum number of lines of code in the test directory
* doc_min_lines: # Minimum number of lines of documentation

Those marked with a % can be expressed as a count, or as a fraction
of the number of lines of source code. For test_min_lines and
doc_min_lines, the denominator of the fraction also includes the
number of lines of test code.
""",
check=(data, guideline_parameters) ->
linecounts_meet_thresholds(data.pkg_code_path, guideline_parameters)
)

function linecounts_meet_thresholds(pkg_code_path,
guideline_parameters)
analysis = PackageAnalyzer.analyze(pkg_code_path;)
# @info analysis
if isempty(analysis.lines_of_code)
return false, "PackageAnalyzer didn't find any lines of code."
end
# get parameters:
src_min_lines = get(guideline_parameters, :src_min_lines, 10)
readme_min_lines = get(guideline_parameters, :readme_min_lines, 5)
test_min_lines = get(guideline_parameters, :test_min_lines, 0.1f0)
doc_min_lines = get(guideline_parameters, :doc_min_lines, 0.1f0)
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved
issues = []
src_line_count = PackageAnalyzer.count_julia_loc(analysis, "src")
if !meets_threshold(readme_min_lines,
PackageAnalyzer.count_readme(analysis),
src_line_count)
push!(issues, "Package README file is too short")
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved
end
if !meets_threshold(src_min_lines, src_line_count)
push!(issues, "Too few lines of source code.")
end
test_line_count = PackageAnalyzer.count_julia_loc(analysis, "test")
if !meets_threshold(test_min_lines,
test_line_count,
test_line_count + src_line_count)
push!(issues, "Too few lines of test code.")
end
docs_line_count = PackageAnalyzer.count_docs(analysis)
if !meets_threshold(doc_min_lines,
docs_line_count,
docs_line_count + src_line_count)
push!(issues, "Too few lines of documentation.")
end
if isempty(issues)
return true, ""
else
return false, join(issues, "\n")
end
end

function _run_pkg_commands(
working_directory::String,
pkg::String,
Expand Down Expand Up @@ -982,6 +1050,9 @@ function get_automerge_guidelines(
# the automerge comment. To make the comment easy
# to read, we want this list to be at the end.
(guideline_distance_check, true),
(guideline_linecounts_meet_thresholds,
# PackageAnalyzer constrains Julia version to 1.6 or newer:
VERSION >= v"1.6"),
]
return guidelines
end
Expand Down Expand Up @@ -1014,6 +1085,12 @@ function get_automerge_guidelines(
(guideline_version_has_osi_license, check_license),
(guideline_src_names_OK, true),
(guideline_version_can_be_imported, true),
#= Do we want to check this for new versions?
(guideline_linecounts_meet_thresholds,
# PackageAnalyzer constrains Julia version to 1.6 or newer:
VERSION >= v"1.6")
=#
]
return guidelines
end

19 changes: 19 additions & 0 deletions src/AutoMerge/public.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ RegistryCI.AutoMerge.run(
additional_check_runs = String[],
check_license = true,
public_registries = String["https://github.com/HolyLab/HolyLabRegistry"],
# Parameters for guideline_linecounts_meet_thresholds
src_min_lines::Integer,
readme_min_linges::Union{Integer, AbstractFloat},
test_min_lines::Union{Integer, AbstractFloat},
doc_min_lines::Union{Integer, AbstractFloat},
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved
)
```
"""
Expand Down Expand Up @@ -99,7 +104,20 @@ function run(;
public_registries::Vector{<:AbstractString}=String[],
read_only::Bool=false,
environment_variables_to_pass::Vector{<:AbstractString}=String[],
# Four parameters for guideline_linecounts_meet_thresholds
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Four parameters for guideline_linecounts_meet_thresholds
# Parameters for guideline_linecounts_meet_thresholds

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 update the count to "Seven".

I want to avoid confusion if additional parameters are added without a comment.

src_min_lines,
readme_min_lines,
test_min_lines,
doc_min_lines,
)::Nothing
# guideline_parameters is for parameters that are passed directly
# from run through to the various Guidelines without modification:
guideline_parameters = Dict{Symbol, Any}(
:src_min_lines => src_min_lines,
:readme_min_lines => readme_min_lines,
:test_min_lines => test_min_lines,
:doc_min_lines => doc_min_lines,
)
all_statuses = deepcopy(additional_statuses)
all_check_runs = deepcopy(additional_check_runs)
push!(all_statuses, "automerge/decision")
Expand Down Expand Up @@ -140,6 +158,7 @@ function run(;
pr_head_commit_sha = current_pr_head_commit_sha(cicfg; env=env)
pull_request_build(
api,
guideline_parameters,
pr_number,
pr_head_commit_sha,
registry_repo,
Expand Down
10 changes: 7 additions & 3 deletions src/AutoMerge/pull_requests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ end

function pull_request_build(
api::GitHub.GitHubAPI,
guideline_parameters::Dict{Symbol, Any},
pr_number::Integer,
current_pr_head_commit_sha::String,
registry::GitHub.Repo,
Expand Down Expand Up @@ -163,12 +164,15 @@ function pull_request_build(
read_only=read_only,
environment_variables_to_pass=environment_variables_to_pass,
)
pull_request_build(data; check_license=check_license)
pull_request_build(data, guideline_parameters;
check_license=check_license)
rm(registry_master; force=true, recursive=true)
return nothing
end

function pull_request_build(data::GitHubAutoMergeData; check_license)::Nothing
function pull_request_build(data::GitHubAutoMergeData,
guideline_parameters::Dict{Symbol, Any},;
check_license)::Nothing
kind = package_or_version(data.registration_type)
this_is_jll_package = is_jll_name(data.pkg)
@info(
Expand Down Expand Up @@ -208,7 +212,7 @@ function pull_request_build(data::GitHubAutoMergeData; check_license)::Nothing
)
end
else
check!(guideline, data)
check!(guideline, data, guideline_parameters)
@info(
guideline.info,
meets_this_guideline = passed(guideline),
Expand Down
26 changes: 23 additions & 3 deletions src/AutoMerge/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,30 @@ struct GitHubAutoMergeData

# Environment variables to pass to the subprocess that does `Pkg.add("Foo")` and `import Foo`
environment_variables_to_pass::Vector{String}

# Parameters for the linecounts_meet_thresholds guideline:
src_min_lines::Integer
readme_min_lines::Union{Integer, AbstractFloat}
test_min_lines::Union{Integer, AbstractFloat}
doc_min_lines::Union{Integer, AbstractFloat}
end

# Constructor that requires all fields (except `pkg_code_path`) as named arguments.
function GitHubAutoMergeData(; kwargs...)
pkg_code_path = mktempdir(; cleanup=true)
kwargs = (; pkg_code_path=pkg_code_path, kwargs...)
fields = fieldnames(GitHubAutoMergeData)
always_assert(Set(keys(kwargs)) == Set(fields))
mismatch = setdiff(Set(keys(kwargs)), Set(fields))
if !isempty(mismatch)
@warn "Keyword/field mismatch while constructing a GitHubAutoMergeData: $(join(mismatchmissing, ", "))"
end
always_assert(isempty(mismatch))
always_assert(kwargs[:authorization] ∈ (:normal, :jll))
return GitHubAutoMergeData(getindex.(Ref(kwargs), fields)...)
end

ALL_GUIDELINES = []
MarkNahabedian marked this conversation as resolved.
Show resolved Hide resolved

Base.@kwdef mutable struct Guideline
# Short description of the guideline. Only used for logging.
info::String
Expand All @@ -130,10 +142,18 @@ Base.@kwdef mutable struct Guideline

# Saved output message from the `check` function.
message::String = "Internal error. A check that was supposed to run never did: $(info)"

function Guideline(info, docs, check, passed, message)
g = new(info, docs, check, passed, message)
push!(ALL_GUIDELINES, g)
g
end
end

passed(guideline::Guideline) = guideline.passed
message(guideline::Guideline) = guideline.message
function check!(guideline::Guideline, data::GitHubAutoMergeData)
return guideline.passed, guideline.message = guideline.check(data)
function check!(guideline::Guideline, data::GitHubAutoMergeData,
guideline_parameters::Dict{Symbol, Any},)
return guideline.passed, guideline.message =
guideline.check(data, guideline_parameters)
end
Loading