-
Notifications
You must be signed in to change notification settings - Fork 74
Hi everyone,
with Phase 1 wrapping up soon, the struct rename test bundle is in review at #2273 and the cascade tests for StructProducer interface variables and STRUCT_DEMUX instance configuration are at #2516, both based on the architecture I confirmed in discussion #2462. Before I start the Phase 2 (FB type rename across nested SubApps, + the folder rename and duplicate name error path coverage from section 2.4 of the proposal), I would like to confirm 3 points.
The 1st question is about driving the folder rename path headlessly, as Phase 1 exercises createChange by performing an LTK IFile rename via RenameResourceDescriptor, and that path activates RenameTypeRefactoringParticipant#createChange cleanly. Phase 2 needs to exercise createPreChange, which only returns a composite change when the renamed element is an IFolder and otherwise returns null. Is the right approach to construct a RenameResourceDescriptor with the folder's IPath and run it through the same PerformChangeOperation machinery, relying on LTK to dispatch it to the participant as an IFolder rename?
The 2nd question is about BlockTypeInstanceSearch and what counts as a nested SubApp instance for the FB rename cascade. Section 2.4 of my proposal targets a fixture with the renamed FB type instantiated at the top-level Application network and inside at least 1 nested SubApp. Reading BlockTypeInstanceSearch and TypeRefactoringHelper.addInstanceChanges, my understanding is that the search walks every FBNetwork reachable from the type library, including the internal FBNetwork of an untyped SubApp instance, and yields each BlockFBNetworkElement whose TypeEntry matches the renamed type, so UpdateFBTypeModelEdit fires once per instance regardless of nesting depth. Could you confirm this is the intended traversal, and clarify whether typed SubApp instances are also returned by the search, or only the FB instances inside them as this determines whether the fixture should contain typed SubApps, untyped SubApps, or both to give meaningful coverage.
The 3rd question is about the LTK lifecycle stage where the duplicate type name validation fires. The proposal includes an error-path test that renames a type to a name already present in the type library and asserts no model edit is applied. My current reading is that the participant's checkConditions populates a RefactoringStatus and a fatal entry there short-circuits createChange, so the test pattern should be to build the CreateChangeOperation with RefactoringStatus.FATAL as the stop level and assert that perform.changeExecuted() returns false while the workspace contents are unchanged. Is checkConditions the location for this validation in the current code, or does the duplicate name check happen elsewhere (eg inside one of the model edit isValid methods) such that the test should assert on a different lifecycle stage?
Thank you in advance for your time and guidance I truly appreciate it.