Repository navigation
Add system tests - #1
Merged
Merged
Conversation
Each PR commit now triggers both the multi-platform build job and a system test job on ubuntu-latest that runs the full 43-test suite.
There was a problem hiding this comment.
Pull request overview
This PR introduces a bash-based system test suite for minibwa and wires it into GitHub Actions so core CLI/indexing/mapping behaviors can be exercised in CI.
Changes:
- Added a comprehensive end-to-end system test runner (
test/run-tests.sh) covering CLI basics, indexing, mapping outputs (SAM/PAF), presets, and several options. - Added additional standalone scripts for utility commands, separate indexing routines, and C API examples (
test/test-*.sh). - Updated CI workflow to run the system tests and updated
.gitignore.
Reviewed changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
test/test-utils.sh |
Adds utility-command tests (bench, fastmap, version). |
test/test-index-routines.sh |
Adds tests for individual indexing pipeline steps (fa2bit, genbwt, genraw, raw2bwt, gensa, etc.). |
test/test-api.sh |
Builds and runs the C API example binaries under api-test/. |
test/run-tests.sh |
Adds the main system test suite used by CI. |
.gitignore |
Ignores .claude. |
.github/workflows/build.yml |
Adds a test job that runs bash test/run-tests.sh. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fail(): accept optional second argument for failure details (both files) - run-tests.sh: register cleanup() via 'trap cleanup EXIT' - run-tests.sh: assert_contains/assert_line_count/assert_not_contains: guard against missing files instead of aborting under set -e - run-tests.sh: version check pattern [0-9]+\.[0-9]+ instead of 0\. - run-tests.sh: genbwt call now passes both input and output paths - test-index-routines.sh: Test 3 passes -t2 to genbwt for real multi-thread - test-index-routines.sh: ls | wc -l pipelines use || echo 0 fallback - test-index-routines.sh: fix raw2bwt comment and output path check - test-index-routines.sh: genraw/raw2bwt tests create .pac via genbwt
- test-bisulfite.sh: - fail() now reports second argument (diagnostic context) - assert_contains/assert_line_count check file existence before grep/wc - Removed redundant tr 'c' 't' (use tr 'Cc' 'Tt' instead) - Fixed assert_contains || assert_contains chain (use single grep) - Fixed assert_contains || pass (use if/else with direct grep) - Fixed mixed-strand FASTA construction (concat decompressed, re-gzip) - Removed duplicate grep in PE flag check - test-utils.sh: - Version check uses [0-9]+ instead of hard-coded 0. for future-proofing
Owner
Author
|
All 19 Copilot review comments have been resolved across two commits: First review (12 comments on run-tests.sh, test-index-routines.sh):
Second review (7 comments on test-bisulfite.sh):
Ready to push once credentials are available. |
Owner
Author
|
All 19 Copilot review comments have been resolved across both reviews. Here is a summary of each fix: Review 4615192533 (5 fixes in run-tests.sh + 7 in test-index-routines.sh)run-tests.sh:
test-index-routines.sh: Review 4621382485 (7 fixes in test-bisulfite.sh)test-bisulfite.sh: All fixes are in the latest commit ( |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Changes:
- Added TMP_DIR=${TMPDIR:-.}/_minibwa_test_tmp to all test scripts
- Replaced all hardcoded /tmp/ paths with $TMP_DIR/ for temp files
- Updated cleanup functions to remove files from $TMP_DIR
- Converted 11 previously skipped tests to actual tests:
* run-tests.sh: 4 tests (chain-only, missing index, missing FASTA, index missing FASTA)
* test-utils.sh: 7 tests (bench 2a, sa, msa, msa -v 50, msa -1, fastmap -n, bench -p, checksum)
- Fixed bench -p test to write to file instead of capturing in variable (1M+ lines)
Result: 175 tests pass, 0 fail, 0 skipped
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+82
to
+85
| "$BINARY" genraw $TMP_DIR/mb_ir_genraw.pac $TMP_DIR/mb_ir_genraw_out > /dev/null 2>&1 | ||
| if [ -f "$TMP_DIR/mb_ir_genraw_out.bwt" ] || [ -f "$TMP_DIR/mb_ir_genraw_out.occ" ]; then | ||
| pass "genraw produces BWT/occ files" | ||
| else |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a comprehensive bash-based system test suite for
minibwaand wires it into GitHub Actions CI, including code coverage reporting.Key Outcomes
System test suite with code coverage
A full bash-based test suite now covers the
minibwaCLI, API, and BS-seq workflows, running on every push/PR via CI.run-tests.sh— CLI basics, indexing, mapping, mem, utilitiestest-utils.sh— bench, fastmap, versiontest-index-routines.sh— Individual indexing steps (fa2bit, genbwt, genraw, raw2bwt, gensa)test-api.sh— C API examples (ex-one.c, ex-batch.c)test-bisulfite.sh— BS-seq simulation testsChanges
Test Suite (
test/)run-tests.sh— Main system test runner covering:test-utils.sh— Utility command tests (bench, fastmap, version)test-index-routines.sh— Individual indexing step teststest-api.sh— C API example teststest-bisulfite.sh— BS-seq simulation tests (43 tests across 16 groups)CI (
.github/workflows/build.yml)testjob that runs all 5 test scripts on every push/PRlcovwith atomic counters (-fprofile-update=atomic)genhtml) and plain-text coverage percentage for shields.io badgeOther
test/data/— moved test fixtures fromtest/into a dedicated data directory.gitignore— ignoretest/_minibwa_test_tmp/Test Coverage
Coverage from CI run: coverage-report.zip
LCOV - code coverage report
Legend: Rating: low: < 75% medium: >= 75% high: >= 90%