Move checkInstallation to the root and speed up its import/export check#637
Merged
Conversation
checkInstallation is the entry point run before RAVEN has been added to the MATLAB path, so it must not depend on any other RAVEN function. It is moved from installation/ to the RAVEN root (where it is easy to find and run on a fresh checkout), its arguments are parsed directly instead of via parseRAVENargs (which would not be on the path yet), and the root is now resolved with a single fileparts of its own location.
Instead of running the full importExportTests test case (which carries the test-framework overhead and compares against a stored reference model), import and export the small "empty" tutorial model directly: import the SBML and YAML files and export the model back to SBML, YAML and Excel, each reported Pass/Fail. This is ~50x faster (about 1 s instead of ~57 s) and no longer depends on the internal ordering of importExportTests, which had shifted when importExcelModel (and its Excel-import test) was removed - the Excel-import check is dropped accordingly, keeping only Excel export.
Function test results201 tests 177 ✅ 35s ⏱️ Results for commit 42d79b3. ♻️ This comment has been updated with latest results. |
Replace the remaining runtests calls (solverTests, blastPlusTests, diamondTests, hmmerTests) with direct checks: solve a trivial LP with each solver, and run each bundled binary with a version/help flag to confirm it executes. This removes checkInstallation's last dependencies on the unit_tests, drops the test-framework overhead and the brittle res() indexing, and is much faster. The now-unused interpretResults helper is removed.
The matlab.unittest function_tests suite (run by CI and runRAVENtests) now covers everything the old xUnit-style testing/unit_tests/ suite did, so the latter is removed. The only unique coverage in unit_tests was the end-to-end ftINIT MILP pipeline in tinitTests.m, which tINIT.m had deferred to. That coverage is migrated into tINIT.m as proper RavenTestCase methods (prepINITModel + ftINIT, gap filling, metabolomics, three-step vs full, and the exact mergeLinear/groupRxnScores/ reverseRxns/rescaleModelForINIT/scoreComplexModel checks), guarded on a MILP solver. The detailed parseTaskList text/Excel comparison is migrated into tTasks.m. Fixtures still used by function_tests (ecoli_textbook.mat, genomeData/, the galactosidase FASTA files, test_tasks.*) and the tINITtestInfo/ model diagrams are relocated to testing/function_tests/test_data and the path references updated; the test fixtures used only by the removed suite are dropped.
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
1 participant
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
Two changes to
checkInstallation, the function users run first on a fresh install.1. Move to the RAVEN root + make it path-independent
installation/checkInstallation.m→checkInstallation.m(root), so it's easy to find and run before RAVEN is on the path.checkInstallationruns before RAVEN has been added to the MATLAB path, so it must not depend on other RAVEN functions. Its arguments are now parsed directly (nargin) instead of viaparseRAVENargs(which wouldn't be found yet), and the root is resolved with a singlefilepartsof its own location. A docstring note explains why it can't useparseRAVENargs.printOrangeis shadowed by a local function, and every other RAVEN call happens after theaddpath, soparseRAVENargswas the only pre-path dependency. Verified withrestoredefaultpaththatcheckInstallation('versionOnly')runs with RAVEN off the path.2. ~50× faster model import/export check
runtests('importExportTests.m')with direct import/export of the smallemptytutorial model (import SBML + YAML, export SBML/YAML/Excel). ~57 s → ~1 s, no test-framework overhead or reference-model comparison.res(1..6)intoimportExportTests, but removingimportExcelModel(and its Excel-import test) shifted those indices. The Excel-import check is dropped (the function no longer exists); Excel export is kept.The only caller (
tInstallation.m) already used positional args, so nothing else needed updating.