Add package folder rename tests and fix struct reference repointing - #2709
Add package folder rename tests and fix struct reference repointing#2709retryoos wants to merge 1 commit into
Conversation
|
@oberlehner ready for review as well when you have some time, this pr adds the package folder rename tests, covering that renaming a folder repackages the contained types and keeps their instances resolvable, while a single file rename only changes the type name |
There was a problem hiding this comment.
Pull request overview
Adds package-folder rename regression tests and supporting fixture data.
Changes:
- Adds folder rename test support.
- Tests package/type renaming and instance resolution.
- Adds a representative 4diac project fixture.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
RefactoringTestSupport.java |
Adds folder rename execution support. |
PackageRenameTestFixture.java |
Defines shared fixture constants. |
PackageFolderRenameTest.java |
Adds package and file rename tests. |
Producer.fbt |
Provides a packaged producer type. |
MyStruct.dtp |
Provides a packaged structured type. |
Consumer.fbt |
Provides a packaged consumer type. |
ControlBlock.fbt |
Provides an unaffected external type. |
PackageRenameTest.sys |
Instantiates and connects packaged types. |
.project |
Defines the importable test project. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Adds headless tests for the package folder rename: renaming a package folder repackages the contained types and keeps their instances resolvable, with an undo and redo round trip, while a single type file rename only changes the type name. Writing the tests surfaced that the folder rename left struct-typed interface pins pointing at the old package. The rename path computed the struct reference target from the type entry's old package name, unlike the move path. It now derives the target from the new path, so the pins repoint to the new package, and the added test guards it. Signed-off-by: Dimitrios Kalligaridis <dimitriskalligaridis@gmail.com>
f727fda to
11a3cea
Compare
retryoos
changed the title
| RefactoringTestSupport.undoLastRefactoring(); | ||
| assertOriginalState(); | ||
|
|
||
| RefactoringTestSupport.redoLastRefactoring(); | ||
| assertRenamedState(); |
There was a problem hiding this comment.
The forward struct-reference repoint is asserted in renamePackageFolder_repointsStructReferencingPins, extending the assertion across the full undo/redo round trip showed that undo reverts the pins correctly, but redo does not re-apply the struct reference to the pins, while the folder, type entry and instance state redo correctly. I want to confirm that redo behavior in the UI before adding round-trip pin coverage or filing it, so for now the round-trip test asserts the state that round-trips correctly and the forward repoint stays in the dedicated test
There was a problem hiding this comment.
🟡 Changes recommended
The acknowledged redo path still fails to reapply the renamed struct references, leaving the round trip incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Adds tests for the package folder rename, and fixes a bug they surfaced.
The tests cover that renaming a package folder repackages the contained types and keeps their instances resolvable, with an undo and redo round trip, while a single type file rename only changes the type name.
While writing them I found that the folder rename left struct-typed interface pins pointing at the old package:
Producer.OUT,Consumer.DIandConsumer.DO1stayedmypackage::MyStructafter renamingmypackagetorenamedpackage. The rename path computed the struct reference target from the type entry's old package name, unlike the move path. It now derives the target from the new path, so the pins repoint to the new package. Verified in the UI on develop and guarded by the newrenamePackageFolder_repointsStructReferencingPinstest.This addresses the two Copilot review comments: the undo/redo round trip is added, and the struct-referencing pins are asserted, which is green now that the underlying bug is fixed.
Closes #2779.