Converged installer, test-harness and tooling conventions on their dominant forms. - #3116
Merged
Merged
Conversation
…g-directory flags.
|
Warning Review limit reached
On-demand reviews are free for the next 12 days. After that, they cost $0.25 per reviewed file. View limit detailsReview configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (188)
Comment |
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3116 +/- ##
==========================================
- Coverage 87.14% 86.77% -0.37%
==========================================
Files 103 101 -2
Lines 5126 5006 -120
Branches 49 3 -46
==========================================
- Hits 4467 4344 -123
- Misses 659 662 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
📖 Documentation preview for this pull request has been deployed to Netlify: https://6aa0000fdf65c241985795bc--vortex-docs.netlify.app This preview is rebuilt on every commit and is not the production documentation site. |
AlexSkrypnyk
added
the
Needs review
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.
No single issue closes this PR - see Follow-ups below for the 11 issues filed from this run (#3105-#3115).
Summary
Every provider closure, handler local, ahoy command key, and BATS assertion in
.vortex/that previously had two or three competing spellings for the same idiom now has one: 195 provider closures across 29 installer test files are hintedAbstractHandlerProcessTestCase $test, theDeployTypes,Modules,CustomModules, andToolshandlers name their prompt-response local$vlike the other 29 handlers, and BATS tests useassert_not_empty,assert_file_not_exists, and"${mocks[@]}"in place of raw[ ]checks and scalar expansion.Eight
.vortex/.ahoy.ymlcommands (install,docs,docs-serve,build-docs,build-installer,lint,lint-fix,test) used aname:key, which ahoy does not read as a description, soahoy --helpprinted no description for any of the eight; everywhere else the same drift was cosmetic, such as a handler-response local named$typesin one file'sprocess()and$vin the next, or a test provider closure hinted three different ways depending on which file it was copied from.ahoy --helpnow prints a description for all eight commands and every other changed file keeps its prior behavior, so noahoy update-snapshotsfixture regeneration is required because the change is confined to.vortex/; two defects surfaced while verifying the pass are filed rather than fixed here - a lowercase strip pattern that never matches the uppercase$TARGET_ENV_REMAPplaceholder invortex-task-purge-cache-acquia(#3105), and three negative BATS assertions innotify-github.batsthat pass while checking a truncated string because they omit the space after the leading-marker (#3106).Before / After
Changes
Installer source (
.vortex/installer/src)HostingProvider::default()returnsself::NONEinstead of the literal'none'string.returninDeployTypes::discover()andNotificationChannels::discover(), and stray blank lines were removed fromDotenvandInternal.Timezone::discover()returnsNULLdirectly instead of through a dead local variable, andExecutableFinderAwareTraitcollapsed to a one-line lazy getter.Strings::isAsciiStart()uses a(bool)cast, and the redundantpublickeyword was dropped from the class constants inRunnerInterfaceandNotificationChannels.DestinationAwareTrait::addOption(), theTaskOutputconstructor, theSchemaValidatorerror array, and aThemecondition); theProfileCustom/ThemeCustomternaries measured at 188 and 195 characters and were deliberately left multi-line.DeployTypes,Modules,CustomModules, andToolshandlers rename their prompt-response local to$vinsideprocess(), matching the other 29 handlers;DeployTypes::discover()'s separate$typeslocal was left alone.Installer tests (
.vortex/installer/tests)DeployTypeHandlerProcessTestwas renamed toDeployTypesHandlerProcessTestto match its handler,tuiTearDown()call casing was corrected to match the trait, a literal'prompts'key was replaced withInstallCommand::OPTION_PROMPTS, anddeclare(strict_types=1)was added toHelpers/TuiOutput.php.useblocks in 32 files were sorted alphabetically, helper methods were moved after the last test method in 5 files, stray blank lines were removed from 3 providers, and a provider was relocated next to the test it feeds inYamlTest.AbstractHandlerProcessTestCase $test(AbstractHandlerDiscoveryTestCaseinBaselineHandlerDiscoveryTest), replacing a 3-way split of untyped,FunctionalTestCase-hinted, and base-class-hinted closures; 7 now-unusedFunctionalTestCaseimports were removed.Test harness (
.vortex/tests)SutTrait::$sutInstallerEnvwas given its array type to match its sibling property,GitTrait::gitReset()was given its string parameter type, the trait-use block inFunctionalTestCasewas alphabetised, and 3 double-quoted strings were requoted to single.ahoy commands and tooling scripts (
.vortex/.ahoy.yml,.vortex/tooling/src).vortex/.ahoy.ymlcommands switched fromname:tousage:, the key ahoy actually reads as a command description.composer --working-dir=Xwas normalised to the space form (composer --working-dir X), andyarn --cwd=./docswas normalised toyarn --cwd=docs.BATS tests (
.vortex/tooling/tests)bats-helpersassertions the project mandates: 11assert_not_emptyinhelpers.batsand 5assert_file_not_existsin the notify shell-injection tests."${mocks}"expansions were changed to"${mocks[@]}", and 107run ./.vortex/...paths were normalised torun .vortex/....stub_sibling()was deduplicated from 3 copies into_helper.bash, and a never-calledsetup_robo_fixture()was removed.Source comments (across
.vortex/).vortex/: diff-narration and what-restating comments were deleted, and survivors were reworded declaratively. Verified mechanically to be comment-only - no code, no printed strings, and no assertion text changed.Follow-ups filed, not fixed here
Eleven issues were filed for what this run deliberately left alone: behaviour-changing convergences, genuine ties with no dominant form, and comments whose claims turned out to be factually wrong. Two of them are latent defects found while verifying this pass, not style choices.
vortex-task-purge-cache-acquiastrips a lowercase$target_env_remap./$target_env.pattern, but the shipped placeholder is uppercase$TARGET_ENV_REMAP., so the prod-domain strip silently never matches; the script has no BATS coverage.notify-github.batsomit the space after the leading-marker, so the parser strips the marker from the wrong offset and the tests pass while checking a truncated string..vortex/CLAUDE.mdno longer matches the shipped boilerplate.passor afail.PromptManagermaintains three separate handler-ordering lists instead of deriving them.