test: trim redundant e2e tests, move pure logic to unit specs - #10651
davidfirst wants to merge 112 commits into
Conversation
PR Summary by QodoTests: trim redundant add/config e2e coverage; add determineMainFile unit specs
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Partial checkouts lose rollback coverage
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
Code review by qodo was updated up to the latest commit 23f127b |
|
Code review by qodo was updated up to the latest commit c9b3b48 |
|
Code review by qodo was updated up to the latest commit d064888 |
|
Code review by qodo was updated up to the latest commit 8d4db88 |
|
Code review by qodo was updated up to the latest commit d15dbd2 |
|
Code review by qodo was updated up to the latest commit ec76886 |
|
Code review by qodo was updated up to the latest commit 0b6f4b3 |
|
Code review by qodo was updated up to the latest commit 42014fb |
resolvePackageNameByPath is pure, so the whole e2e file is replaced by cases in resolve-pkg-name-by-path.spec.ts, which also covers previously untested branches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| it('should save "chai" in the dev-packages', () => { | ||
| expect(comp1.devPackageDependencies).to.be.an('object').that.has.property('chai'); | ||
| }); | ||
| it('should not save "chai" in the packages', () => { | ||
| expect(comp1.packageDependencies).to.be.an('object').that.is.empty; | ||
| }); | ||
| it('should not save anything into dependencies', () => { | ||
| expect(comp1.dependencies).to.be.an('array').that.is.empty; | ||
| }); |
There was a problem hiding this comment.
16. Dev packages can be misclassified 🐞 Bug ≡ Correctness
The consolidated dev-dependency scenario removes assertions that chai appears in devPackageDependencies and not in packageDependencies. Its remaining checks cover component dependency arrays only, so package imports from test files can enter the regular-package bucket without failing this suite.
Agent Prompt
## Issue description
The surviving test no longer verifies that an npm package imported exclusively by a test file is classified as a development package rather than a regular package.
## Fix Focus Areas
- e2e/functionalities/dev-dependencies.e2e.ts[76-94]
- scopes/dependencies/dependencies/dependencies-loader/auto-detect-deps.ts[475-493]
## Recommended Fix
Add both classification assertions to the consolidated test: require `chai` in `devPackageDependencies` and require it to be absent from `packageDependencies`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| it('adds peer dependency to the generated package.json', () => { | ||
| const pkgJson = fs.readJsonSync( | ||
| path.join(workspaceCapsulesRootDir, `${helper.scopes.remote}_comp1/package.json`) | ||
| ); | ||
| expect(pkgJson.peerDependencies).to.deep.equal({ | ||
| [`@${helper.scopes.remote}/comp2`]: '^0.0.1-new', | ||
| }); | ||
| }); | ||
| describe('peer dependency is not broken after snap', () => { | ||
| before(() => { | ||
| helper.command.snapAllComponents(); | ||
| helper.command.build(); | ||
| workspaceCapsulesRootDir = helper.command.capsuleListParsed().workspaceCapsulesRootDir; | ||
| }); | ||
| it('should save the peer dependency in the model', () => { |
There was a problem hiding this comment.
17. Peer manifests can omit dependencies 🐞 Bug ≡ Correctness
The deleted capsule assertions were the only checks that component peer dependencies are emitted into generated package.json files. The surviving model assertions bypass manifest conversion, so capsules can retain correct scope metadata while exposing an incorrect or missing peer dependency to package consumers.
Agent Prompt
## Issue description
Model-level peer dependency assertions do not verify the separate conversion into generated capsule package manifests.
## Fix Focus Areas
- e2e/functionalities/peer-dependency-component.e2e.ts[127-161]
- scopes/dependencies/dependency-resolver/dependencies/dependency-list.ts[155-176]
## Recommended Fix
Keep one focused assertion that builds a capsule, reads its generated package.json, and verifies the component dependency appears under peerDependencies with the expected range.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| @@ -227,12 +211,10 @@ describe('compile extension', function () { | |||
| helper.command.setEnv('comp1', EMPTY_ENV); | |||
There was a problem hiding this comment.
19. Multiple directory compilation loses coverage 🐞 Bug ☼ Reliability
The removed compile case is the only scenario that supplies two component root-directory paths in one command invocation. The retained mixed-input test combines one directory with an ID instead, leaving multi-directory parsing and component selection unverified.
Agent Prompt
Issue description
Compilation with multiple root-directory arguments is no longer tested, although it is a distinct command-input form from a directory plus component ID.
Fix Focus Areas
- e2e/harmony/compile.e2e.ts[185-204]
Recommended Fix
Restore a test that invokes compile with `nested/comp1 nested/comp2` and asserts both components are selected and compiled.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.fs.createFile('bar', file1); | ||
| helper.fs.createFile('bar', file2); | ||
|
|
||
| const addCmd = () => helper.command.addComponent('bar', { n: 'test' }); |
There was a problem hiding this comment.
15. Adding unresolved components goes untested 🐞 Bug ≡ Correctness
The deleted bit add scenario was the only integration test for automatic main-file resolution exhausting every strategy and throwing MissingMainFile. The new unit suite checks a missing user-specified file but not the default unresolved case or its propagation through the add command.
Agent Prompt
## Issue description
The replacement unit suite does not cover failure after automatic main-file resolution exhausts every strategy, and no command test now verifies that error propagation.
## Fix Focus Areas
- scopes/component/tracker/determine-main-file.spec.ts[108-115]
- e2e/commands/add.e2e.ts[72-82]
## Recommended Fix
Add a unit case with multiple files, no explicit main file, and no recognized index or directory-named file, asserting `MissingMainFile`. Retain a focused command-level assertion if propagation through `bit add` is not covered elsewhere.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| expect(output).to.have.string('initialized a bit workspace'); | ||
| }); | ||
|
|
||
| it('should skip interactive mode with --external-package-manager flag', () => { |
There was a problem hiding this comment.
18. Flagged initialization can still prompt 🐞 Bug ☼ Reliability
The deleted initialization cases were the only tests proving that external-package-manager, standalone, and reset modes bypass runInteractiveMode(). These flags reach an explicit short-circuit in the command, and a regression there can make scripted initialization wait for interactive input.
Agent Prompt
## Issue description
Initialization flags intended for scripted operation no longer have coverage proving that they bypass the interactive setup path.
## Fix Focus Areas
- e2e/commands/init.e2e.ts[510-527]
- scopes/harmony/host-initializer/init-cmd.ts[76-125]
## Recommended Fix
Retain a parameterized test for the external-package-manager, standalone, and reset options. Assert that each invocation completes successfully without emitting or entering interactive setup.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| expect(componentFileLocation).to.be.a.file(); | ||
| }); | ||
| }); | ||
| describe('when the destination is an existing empty directory', () => { |
There was a problem hiding this comment.
1. Custom-path imports lose coverage 🐞 Bug ☼ Reliability
The deleted bit import --path destination suite was the only command-level coverage for successful writes to an empty directory, rejection and content preservation for a non-empty directory, overwrite behavior with --override, and rejection when the target is an existing file. These cases reach separate component-writer conflict branches from the surviving non-existent-destination test, so regressions in destination handling and safeguards against overwriting user files no longer fail an integration test.
Agent Prompt
## Issue description
The `bit import --path` destination scenarios were removed even though empty directories, occupied directories, existing file targets, and override behavior follow distinct component-writer paths. Command-level coverage should verify both successful destination handling and safeguards that preserve existing user content.
## Fix Focus Areas
- e2e/commands/import.e2e.ts[160-223]
- scopes/component/component-writer/component-writer.main.runtime.ts[333-358]
## Recommended Fix
Restore focused destination tests that verify import into an empty directory, rejection of a non-empty destination without deleting or changing its existing content, successful replacement of a directory with `--override`, and rejection when the destination is a file both with and without `--override`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // Note: getClonedWorkspace destroys the current dir and restores from the snapshot, | ||
| // so each runAsFirstCommand call starts fresh from the same pre-CI state. | ||
|
|
||
| it('`bit status` as first command syncs bitmap', () => { |
There was a problem hiding this comment.
20. Status no longer checks automatic sync 🐞 Bug ☼ Reliability
The PR deletes the first-command scenario that invokes helper.command.status() after the CI merge. The retained first-command tests cover list, show, log, and compile, so a status-specific failure to trigger bitmap synchronization is no longer detected.
Agent Prompt
Issue description
The suite no longer verifies that status triggers bitmap synchronization when it is the first Bit command after the CI merge.
Fix Focus Areas
- e2e/harmony/ci-bitmap-auto-sync.e2e.ts[340-372]
Recommended Fix
Restore the first-command status test using `runAsFirstCommand(() => helper.command.status())` and assert the bitmap records version `0.0.2`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.scopeHelper.addRemoteScope(); | ||
| helper.command.importLane('dev'); | ||
|
|
||
| helper.fs.outputFile(`${helper.scopes.remote}/comp3/index.js`, 'console.log("v2");'); |
There was a problem hiding this comment.
21. Pending merges can update other components 🐞 Bug ☼ Reliability
The deleted pending-merge scenario uniquely creates a pending merge through a main-to-lane merge and then publishes a changed unrelated component from another workspace before calling checkout. The retained test only compares component heads after a different import-based setup, so it no longer verifies that checkout leaves an unrelated component's files untouched when it aborts.
Agent Prompt
Issue description
Checkout failure with a pending merge is no longer tested when another component has a newly published remote file revision.
Fix Focus Areas
- e2e/harmony/lanes/bit-checkout-on-lanes.e2e.ts[78-115]
- e2e/harmony/lanes/bit-checkout-on-lanes.e2e.ts[15-45]
Recommended Fix
Restore the scenario that creates a pending merge for one component, exports a changed unrelated component from a second workspace, runs checkout, and asserts the unrelated workspace file was not updated.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| }); | ||
| describe('rename lane using the alias', () => { | ||
| before(() => { | ||
| helper.scopeHelper.setWorkspaceWithRemoteScope(); |
There was a problem hiding this comment.
22. Aliased lane renames lose coverage 🐞 Bug ☼ Reliability
The deleted test creates and exports a lane with --alias before renaming it, whereas the purported replacement creates an ordinary lane. Alias creation is otherwise only exercised when switching to remote lanes, so a rename regression involving alias-backed lane state is no longer covered.
Agent Prompt
Issue description
Renaming an exported lane created with an alias is no longer tested; the retained exported-lane test does not use an alias.
Fix Focus Areas
- e2e/harmony/lanes/rename-lane.e2e.ts[16-33]
- e2e/harmony/lanes/lane-management.e2e.ts[18-52]
Recommended Fix
Restore the aliased-lane scenario: create `dev` with `--alias d`, snap and export it, rename it, then assert status succeeds and `.bitmap` contains the new lane name.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit a244936 |
# Conflicts: # .circleci/config.yml
…tester-spec-files Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
||
| helper.scopeHelper.getClonedWorkspace(originalWs); | ||
| }); | ||
| it('checkout head should stop with an error', () => { |
There was a problem hiding this comment.
14. Forced checkout can update unrelated components 🐞 Bug ☼ Reliability
The deleted pending-merge scenario was the only one to call checkoutHead('-x') after a
merge-pending component was created and to verify that another component did not advance. The
surviving pending-merge setup invokes checkoutHead() without the force flag, while its only forced
checkout scenario has no merge-pending component, so the force-specific safeguard is no longer
exercised.
Agent Prompt
Issue description
The forced `checkoutHead('-x')` path is no longer tested when a workspace contains a merge-pending component. Preserve the assertion that the command fails and does not advance an unrelated component.
Fix Focus Areas
- e2e/harmony/lanes/bit-checkout-on-lanes.e2e.ts[78-115]
Recommended Fix
Restore the deleted pending-merge setup and assertions, or add an equivalent isolated scenario that invokes `checkoutHead('-x')`, expects the merge-pending failure, and verifies that the unrelated component remains at its original revision.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.fs.deletePath('comp1/foo.js'); | ||
| helper.command.snapAllComponentsWithoutBuild(); | ||
| helper.command.export(); | ||
| helper.command.switchLocalLane('main', '-x'); |
There was a problem hiding this comment.
15. Fast-forward merges can retain deleted files 🐞 Bug ≡ Correctness
The PR deletes the only scenario in merge-lanes-edge-cases.e2e.ts that merges a lane containing a file deletion into unchanged main, which exercises direct remote-component application without three-way results. Because the retained scenario creates a new main tag before merging, it covers only the diverged three-way path, allowing obsolete workspace files left by fast-forward merges to escape the suite.
Agent Prompt
## Issue description
The cleanup removes the only integration scenario verifying that a fast-forward lane merge deletes files removed on the source lane. The retained scenario deliberately diverges main before merging and therefore exercises the separate three-way merge path, which cannot cover regressions in direct remote-component application.
## Fix Focus Areas
- e2e/harmony/lanes/merge-lanes-edge-cases.e2e.ts[379-405]
## Recommended Fix
Restore an isolated non-diverged integration scenario: leave main at the common base, remove and snap the file on the source lane, switch to unchanged main, merge the lane, and verify both the removal status in the command output and the file's absence from the workspace.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| }); | ||
| }); | ||
|
|
||
| describe('when no components in workspace', () => { |
There was a problem hiding this comment.
16. Empty workspace searches can misreport results 🐞 Bug ≡ Correctness
The deleted when no components in workspace block was the only command test that reinitialized an empty workspace before invoking local-only search. All surviving search cases populate and tag components first, even though SearchCmd.report() has a dedicated zero-local-results rendering branch for workspaces.
Agent Prompt
Issue description
The suite no longer executes `bit search --local-only` in an empty workspace. Preserve a test for the empty-workspace local output path.
Fix Focus Areas
- e2e/commands/search.e2e.ts[73-80]
Recommended Fix
Add a separate describe that reinitializes an empty workspace, runs `bit search anything --local-only`, and asserts that the local no-match message is rendered.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const statusOutput = helper.command.runCmd('bit status'); | ||
| expect(statusOutput).to.have.string('modified components'); | ||
| }); | ||
| it('should update bitmap with the imported version', () => { |
There was a problem hiding this comment.
17. Manual merges can retain the old version 🐞 Bug ≡ Correctness
The PR removes the bitmap-version assertion from the manual conflict merge branch, leaving that branch checked only for conflict markers and modified status. A manual merge can therefore leave comp2 recorded at its prior revision while still satisfying every remaining assertion in that branch.
Agent Prompt
Issue description
The manual merge scenario no longer verifies that importing revision `0.0.2` updates the component revision stored in the bitmap. Keep this bookkeeping assertion alongside the conflict-content checks.
Fix Focus Areas
- e2e/functionalities/merge.e2e.ts[111-114]
Recommended Fix
After the manual merge completes, read the bitmap and assert that `comp2.version` equals the imported version. Keep the assertion specific to the manual conflict strategy.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| expect(peerDep.version).to.eq('latest'); | ||
| expect(peerDep.versionRange).to.eq('^0.0.1-new'); | ||
| }); | ||
| it('adds peer dependency to the generated package.json', () => { |
There was a problem hiding this comment.
18. Built packages can omit peer dependencies 🐞 Bug ≡ Correctness
The PR deletes the capsule package.json assertions after setPeer('comp2', '^') and after the
subsequent snap, leaving only model and scope metadata checks. Capsule manifest generation
separately maps dependency lifecycle entries into peerDependencies, so persistence of the model
data does not establish that the generated package contains the required peer range.
Agent Prompt
Issue description
The `set-peer` scenarios no longer verify the peer dependency emitted into the generated capsule package manifest. Retain checks before and after snapping so model metadata and generated package output stay aligned.
Fix Focus Areas
- e2e/functionalities/peer-dependency-component.e2e.ts[154-161]
- e2e/functionalities/peer-dependency-component.e2e.ts[194-202]
Recommended Fix
Read the relevant workspace capsule `package.json` after each build and assert that `peerDependencies` contains the component package with the expected range.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 1f81463 |
| @@ -107,12 +82,6 @@ describe('bit add command', function () { | |||
| const mainFile = bitMap['test/bar'].mainFile; | |||
| expect(mainFile).to.equal('bar.js'); | |||
| }); | |||
| it('Should return error if used an invalid ID', () => { | |||
| helper.fs.createFile('bar', 'foo.js'); | |||
| const addFunc = () => helper.command.addComponent('bar', { i: 'Bar/foo' }); | |||
| const error = new InvalidName('Bar/foo'); | |||
| helper.general.expectToThrow(addFunc, error); | |||
| }); | |||
| it('should add component with id contains only one level', () => { | |||
| helper.fs.createFile('bar', 'foo.js'); | |||
| helper.command.addComponent('bar', { | |||
| @@ -122,20 +91,6 @@ describe('bit add command', function () { | |||
| expect(bitMap).to.have.property('foo'); | |||
There was a problem hiding this comment.
17. Malformed component names lose command coverage 🐞 Bug ☼ Reliability
addOneComponent() sends an explicitly supplied component name through BitId.parse(..., false), but the PR removes the command cases asserting malformed dotted names throw InvalidName. The surviving add tests use valid supplied names or the mutually-exclusive-option error, so a validation regression in the explicit-name CLI path reaches users without an integration failure.
Agent Prompt
Issue description
The PR removes active `bit add` cases for malformed explicit component names. Valid-name and flag-conflict tests do not exercise the parser validation path used by `--id`.
Fix Focus Areas
- e2e/commands/add.e2e.ts[88-99]
- scopes/component/tracker/add-components.ts[491-525]
Recommended Fix
Restore a focused command-level test for malformed explicit names, including dotted path segments, and assert the command raises `InvalidName`. A table-driven test can cover both removed invalid-name inputs without duplicating setup.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| it('checkout head should stop with an error', () => { | ||
| expect(() => helper.command.checkoutHead('-x')).to.throw(); | ||
| }); | ||
| it('should not merged the head of other components', () => { |
There was a problem hiding this comment.
1. Partial checkouts lose rollback coverage 📘 Rule violation ☼ Reliability
Deleting checkout when some are pending-merge removes the only filesystem assertion that checkoutHead() leaves other components untouched after an error. When one lane component is pending merge and another has a newer remote head, the surviving bitmap-only test cannot detect files written before the operation aborts.
Agent Prompt
## Issue description
The deleted checkout scenario uniquely verifies that a failed multi-component checkout does not partially update another component's files.
## Fix Focus Areas
- e2e/harmony/lanes/bit-checkout-on-lanes.e2e.ts[15-40]
## Recommended Fix
Add a minimal E2E scenario with one merge-pending component and one component with a newer remote head. Assert that checkout fails and that the unaffected component's file content remains unchanged.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.fixtures.fs.readJsonFile(`node_modules/@ci/${randomStr}.comp2/package.json`).componentId.version | ||
| ).to.equal('0.0.2'); | ||
| }); | ||
| it('should not nest/install the version from the component model to the component node_modules dir', () => { |
There was a problem hiding this comment.
2. Dependency updates lose dedupe coverage 📘 Rule violation ≡ Correctness
Deleting simple scenario removes the export-import-update flow that verifies a newer workspace-policy dependency is hoisted without retaining the older model version below the component. When a dependent is imported after its dependency receives a newer version, the remaining complex graph expects an older policy version to stay nested and therefore does not exercise this replacement path.
Agent Prompt
## Issue description
The removed scenario uniquely covers deduplication when a workspace installs a newer dependency than the version recorded in an imported component model.
## Fix Focus Areas
- e2e/harmony/deduplication.e2e.ts[28-68]
## Recommended Fix
Restore a minimal export-import-update scenario that asserts the newer version exists at the workspace root and the stale model version is not installed beneath the dependent component.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.scopeHelper.getClonedWorkspace(localBeforeTag); | ||
| const componentShowBeforeRemove = helper.command.showComponentParsed('bar/foo'); | ||
| dummyExtensionBefore = findDummyExtension(componentShowBeforeRemove.extensions); | ||
| helper.extensions.addExtensionToVariant( |
There was a problem hiding this comment.
3. Extension removal loses coverage 📘 Rule violation ≡ Correctness
Deleting removing the extension with "-" removes the only E2E scenario that applies the - override through variant configuration and verifies that an inherited extension disappears from the resolved component configuration. Because this override exercises dedicated removal-marker behavior, the surviving add, persistence, import, tag, and export tests do not cover regressions in component-level extension suppression when a workspace masks a configured extension.
Agent Prompt
## Issue description
The deleted scenario uniquely provided end-to-end coverage for suppressing an inherited extension through real workspace variant configuration using the special `-` removal marker.
## Fix Focus Areas
- e2e/harmony/extensions-config.e2e.ts[207-210]
- scopes/workspace/workspace/bit-map.ts[101-115]
- scopes/workspace/workspace/component-config-file/component-config-file.ts[130-143]
## Recommended Fix
Restore a focused E2E scenario that configures an extension on a component, applies the `-` override to that inherited extension through the variant, and verifies through the resolved component output that the extension is absent.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| expect(output).to.have.string('initialized a bit workspace'); | ||
| }); | ||
|
|
||
| it('should skip interactive mode with --external-package-manager flag', () => { |
There was a problem hiding this comment.
4. Init flags lose prompt-bypass coverage 📘 Rule violation ☼ Reliability
Deleting the interactive-mode flag cases leaves init() covered only for the default prompt path and --skip-interactive, even though handleInteractiveMode() has separate bypass conditions for external package manager, standalone, and reset options. If option parsing or any bypass guard regresses, automated initialization in an existing Git repository can reach the interactive setup prompt without the surviving command-level suite detecting it.
Agent Prompt
## Issue description
The PR removes command-level coverage showing that external-package-manager, standalone, and reset options bypass interactive setup when initialization runs in an existing Git repository. The remaining `--skip-interactive` test cannot detect regressions in these separate option-parsing and guard paths.
## Fix Focus Areas
- e2e/commands/init.e2e.ts[424-462]
- e2e/commands/init.e2e.ts[510-514]
- scopes/harmony/host-initializer/init-cmd.ts[82-105]
## Recommended Fix
Add a compact parameterized E2E test that initializes an otherwise uninitialized Git repository with each external-package-manager, standalone, and reset option. Assert that every invocation succeeds without emitting the interactive setup prompt, and keep the cases isolated using the existing workspace cleanup hook.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const ramdaDep = showConfig.data.dependencies.find((d) => d.id === 'ramda'); | ||
| expect(ramdaDep.version).to.equal('0.0.21'); | ||
| }); | ||
| it('running bit deps set of another pkg, should work', () => { |
There was a problem hiding this comment.
14. Post-conflict dependency edits untested 🐞 Bug ⚙ Maintainability
The deleted merge-config assertion was the only test calling dependenciesSet after
fixMergeConfigConflict('theirs'), while the replacement comment points to a scenario that sets
dependencies before merging. Conflict resolution leaves component-specific configuration in a
distinct state, so failures to merge a subsequent dependency policy into that state can now pass
unnoticed.
Agent Prompt
## Issue description
The cleanup incorrectly treats dependency setup before a merge as equivalent to editing dependencies after resolving a merge-config conflict.
## Fix Focus Areas
- e2e/harmony/merge-config.e2e.ts[342-366]
- scopes/dependencies/dependencies/dependencies.main.runtime.ts[168-202]
## Recommended Fix
Restore a focused assertion after `fixMergeConfigConflict('theirs')` that runs `dependencies set` and verifies the new package is merged into the resolved component policy.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| helper.fs.deletePath('comp1/foo.js'); | ||
| helper.command.snapAllComponentsWithoutBuild(); | ||
| helper.command.export(); | ||
| helper.command.switchLocalLane('main', '-x'); |
There was a problem hiding this comment.
15. Fast-forward deletions can regress 🐞 Bug ⚙ Maintainability
The deleted lane scenario was the only test merging a file deletion while main remained at the common base, which takes the non-diverged fast-forward path. The retained scenario advances main first and performs a three-way merge, so its identical output and filesystem assertions do not exercise the removed merge path.
Agent Prompt
## Issue description
The retained diverged-lane test does not cover applying a deleted file through the fast-forward merge path.
## Fix Focus Areas
- e2e/harmony/lanes/merge-lanes-edge-cases.e2e.ts[379-406]
## Recommended Fix
Keep a compact non-diverged scenario that deletes a tracked file on the lane, merges without advancing main, and verifies both removal output and filesystem deletion.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| describe('the referenced snap is present locally in the merging workspace', () => { | ||
| let laneScopeName: string; | ||
| before(() => { | ||
| ({ laneScopeName } = setupLaneWithStaleRef()); |
There was a problem hiding this comment.
16. Local stale-reference exports untested 🐞 Bug ⚙ Maintainability
The deleted stale-reference scenario merged and exported while the referenced squashed-out snap was already present in the workspace object store. The surviving same-scope scenario deliberately starts without that object and imports it during merge, while the other survivor uses cross-scope integrity semantics, leaving the locally available export path uncovered.
Agent Prompt
## Issue description
The cleanup removes the only same-scope stale-reference export where the pinned snap already exists locally before lane merge.
## Fix Focus Areas
- e2e/harmony/lanes/merge-lane-to-main-stale-dep-ref.e2e.ts[58-160]
## Recommended Fix
Restore a focused same-scope case using the existing setup helper, merge from the original workspace where the stale snap is present, and assert that exporting the merged result succeeds.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 96f98d3 |
1 participant
This PR reduces the number of slow e2e tests. The work goes file by file. Each commit covers one e2e file.
Done so far: add.e2e.ts (24 to 13 tests), config.e2e.ts (13 to 11 tests). More commits will follow on this branch.