Skip to content

Add testthat coverage and package contributor guidance - #17

Merged
asgr merged 3 commits into
masterfrom
copilot/create-unit-tests-and-update-ci
Nov 6, 2026
Merged

asgr merged 3 commits into
masterfrom
copilot/create-unit-tests-and-update-ci

Conversation

Copilot AI commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Add focused unit tests for Highlander’s sampling helpers and algorithm-spec generation, and document package structure and contribution guidance. Reject invalid Lowlander() sample counts; the existing R CMD-check workflow already runs testthat.

  • Tests: Cover deterministic search behavior, likelihood direction, helper ranking and seeding, Specs generation, and input validation.
  • Validation: Reject non-positive, fractional, and out-of-range Nsamp values.
  • Guidance: Add package-specific AGENTS.md with test and check commands.

Copilot AI and others added 2 commits October 31, 2026 06:36
Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>
Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>
@asgr

asgr commented Nov 6, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

…-tests-and-update-ci

# Conflicts:
#	tests/testthat/test-lowlander-helpers.R

Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>

Copilot AI commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved and merged master; the conflict resolution is in merge commit 491f50a.

@asgr
asgr marked this pull request as ready for review November 6, 2026 05:27
Copilot AI balanced review requested due to automatic review settings November 6, 2026 05:27
@asgr
asgr merged commit 5c521c3 into master Nov 6, 2026
4 checks passed
@asgr
asgr deleted the copilot/create-unit-tests-and-update-ci branch November 6, 2026 05:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The Nsamp tests lack a case for .Machine$integer.max + 1, leaving the new upper-bound guard unverified and its removal undetected.

1 open issue
What changed in this PR

This PR expands Highlander’s unit coverage, strengthens Lowlander() sample-count validation, and adds package-specific contributor guidance.

Changes:

  • Add focused tests for sampling, helper ranking and seeding, likelihood conversion, and algorithm settings.
  • Reject non-positive, fractional, and out-of-range Nsamp values.
  • Document package structure, test commands, and contribution expectations.
File Description
tests/​testthat/​test-specs-help.R Test algorithm defaults, ensemble initialization, and grid bounds.
tests/​testthat/​test-lowlander.R Test deterministic search, likelihood direction, and input validation.
tests/​testthat/​test-lowlander-helpers.R Expand ranking, starting-point, and invalid-input coverage.
tests/​testthat/​test-likelihood-converters.R Test optimization direction and prior handling.
tests/​testthat/​helper-lowlander-fixture.R Add a shared deterministic fixture.
R/​Lowlander.R Strengthen sample-count validation before integer conversion.
AGENTS.md Document package layout and contributor workflows.

Review effort: Balanced

Comment on lines +70 to +73
expect_error(
do.call(Lowlander, utils::modifyList(args, list(Nsamp = 1.5))),
"Nsamp must be a positive integer"
)
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.

3 participants