Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple tests with refactoring driver for duplicate class. #16853

Merged
merged 9 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/Refactoring-Core/ReCopyClassRefactoring.class.st
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
"
I am a refactoring for copy a class.

My preconditions verify, that the copied class exists (in the current namespace) and that the new copy class name is valid and not yet used as a global variable name
My preconditions verify, that the copied class exists (in the current namespace) and that the new copy class name is valid and not yet used as a global variable name.

The refactoring transformation create a new class and copy all instance and class methods of copied class.

# Example
```smalltalk
Example
---------------
```
(ReCopyClassRefactoring
copyClass: #RBFooLintRuleTestData
withName: #RBFooLintRuleTestData1 in: #Example1) execute.
```

## Implementation

- className refers to a <Symbol> representing the new class to be created.
- aClass is the RefactoringBrowser representation of className.

"
Class {
#name : 'ReCopyClassRefactoring',
#superclass : 'RBClassRefactoring',
#instVars : [
'aClass',
'packageName',
'instanceMethods',
'classMethods'
'classMethods',
'instanceMethods'
],
#category : 'Refactoring-Core-Refactorings',
#package : 'Refactoring-Core',
Expand Down Expand Up @@ -68,6 +75,7 @@ ReCopyClassRefactoring >> applicabilityPreconditions [
ReCopyClassRefactoring >> classMethods [

^ classMethods
ifNil: [ classMethods := aClass classSide allMethods ].
]

{ #category : 'accessing' }
Expand All @@ -88,11 +96,12 @@ ReCopyClassRefactoring >> copyClass [

{ #category : 'copying' }
ReCopyClassRefactoring >> copyClass: cls withName: aName [

self className: aName.
aClass := self classObjectFor: cls
]

{ #category : 'transforming' }
{ #category : 'copying' }
ReCopyClassRefactoring >> copyMethods [
| newClass |
newClass := (self model classNamed: className).
Expand Down Expand Up @@ -126,7 +135,7 @@ ReCopyClassRefactoring >> copyMethodsOf: rbClass1 in: rbClass2 [
]
]

{ #category : 'transforming' }
{ #category : 'copying' }
ReCopyClassRefactoring >> copySelectedMethods [
| newClass |

Expand Down Expand Up @@ -154,6 +163,7 @@ ReCopyClassRefactoring >> copyVariables [
ReCopyClassRefactoring >> instanceMethods [

^ instanceMethods
ifNil: [ instanceMethods := aClass methods ]
]

{ #category : 'accessing' }
Expand Down Expand Up @@ -183,7 +193,7 @@ ReCopyClassRefactoring >> privateTransform [

]

{ #category : 'copying' }
{ #category : 'initialize' }
ReCopyClassRefactoring >> sourceClass: classToCopy [

aClass := self classObjectFor: classToCopy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ Class {
#tag : 'Manifest'
}

{ #category : 'code-critics' }
ManifestRefactoringDataForTesting class >> ruleNoUnusedInstanceVariableRuleV1FalsePositive [

<ignoreForCoverage>
^ #(#(#(#RGClassDefinition #(#ReClassForGeneratingEqualAndHash)) #'2024-07-05T12:05:01.988597+02:00') #(#(#RGClassDefinition #(#ReClassForGeneratingEqualAndHashExistingImplementors)) #'2024-07-05T12:07:30.662537+02:00') )
]

{ #category : 'code-critics' }
ManifestRefactoringDataForTesting class >> ruleReIvarNeitherReadNorWrittenRuleV1FalsePositive [

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ RBClassDataForRefactoringTest >> sendsDifferentSuper [
{ #category : 'lint' }
RBClassDataForRefactoringTest >> sizeCheck [

self isEmpty
self isEmpty
ifFalse: [ self do: [ :each | each traceCr ] ]
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,27 @@ Class {
#package : 'Refactoring-DataForTesting',
#tag : 'ForGeneratingEqualAndHash'
}

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHash >> iVar1 [

^ iVar1
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHash >> iVar1: anObject [

iVar1 := anObject
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHash >> iVar2 [

^ iVar2
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHash >> iVar2: anObject [

iVar2 := anObject
]
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Class {
#name : 'ReClassForGeneratingEqualAndHashExistingImplementors',
#superclass : 'Object',
#instVars : [
'iVarA',
'name',
'iVarB'
],
#category : 'Refactoring-DataForTesting-ForGeneratingEqualAndHash',
Expand All @@ -16,12 +16,36 @@ ReClassForGeneratingEqualAndHashExistingImplementors >> = anObject [

self == anObject ifTrue: [ ^ true ].
self class = anObject class ifFalse: [ ^ false ].
^ iVarA = anObject iVarA
^ self name = anObject name
]

{ #category : 'comparing' }
ReClassForGeneratingEqualAndHashExistingImplementors >> hash [
"Answer an integer value that is related to the identity of the receiver."

^ iVarA hash
^ self name hash
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHashExistingImplementors >> iVarB [

^ iVarB
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHashExistingImplementors >> iVarB: anObject [

iVarB := anObject
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHashExistingImplementors >> name [

^ name
]

{ #category : 'accessing' }
ReClassForGeneratingEqualAndHashExistingImplementors >> name: anObject [

name := anObject
]
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,27 @@ Class {
#package : 'Refactoring-DataForTesting',
#tag : 'ForGeneratingPrintOn'
}

{ #category : 'accessing' }
ReClassForGeneratingPrintOn >> iVarA [

^ iVarA
]

{ #category : 'accessing' }
ReClassForGeneratingPrintOn >> iVarA: anObject [

iVarA := anObject
]

{ #category : 'accessing' }
ReClassForGeneratingPrintOn >> iVarB [

^ iVarB
]

{ #category : 'accessing' }
ReClassForGeneratingPrintOn >> iVarB: anObject [

iVarB := anObject
]
19 changes: 19 additions & 0 deletions src/Refactoring-DataForTesting/ReClassToBeDuplicated.class.st
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
Class {
#name : 'ReClassToBeDuplicated',
#superclass : 'Object',
#instVars : [
'instVar1'
],
#category : 'Refactoring-DataForTesting-ForDuplication',
#package : 'Refactoring-DataForTesting',
#tag : 'ForDuplication'
}

{ #category : 'comparing' }
ReClassToBeDuplicated >> = anObject [
"Answer whether the receiver and anObject represent the same object."

self == anObject ifTrue: [ ^ true ].
self class = anObject class ifFalse: [ ^ false ].
^ instVar1 = anObject instVar1
]

{ #category : 'comparing' }
ReClassToBeDuplicated >> hash [
"Answer an integer value that is related to the identity of the receiver."

^ instVar1 hash
]
47 changes: 32 additions & 15 deletions src/Refactoring-UI-Tests/ReDuplicateClassDriverTest.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ Class {
#tag : 'Driver'
}

{ #category : 'tests' }
ReDuplicateClassDriverTest >> classToBeDeleted [

^ #ReClassCopiedToBeDeleted
]

{ #category : 'tests' }
ReDuplicateClassDriverTest >> classToBeDuplicated [
"Answer the <Class> which will be duplicated"
Expand All @@ -19,7 +25,7 @@ ReDuplicateClassDriverTest >> classToBeDuplicated [
{ #category : 'tests' }
ReDuplicateClassDriverTest >> setUpMocksOn: driver [

| dialog |
| dialog requestClass |
dialog := MockObject new.
dialog
on: #window respond: SpDialogWindowPresenter new beOk;
Expand All @@ -28,38 +34,49 @@ ReDuplicateClassDriverTest >> setUpMocksOn: driver [
on: #openModal respond: dialog.
driver requestDialog: dialog.
driver configureRefactoring.

requestClass := MockObject new.
requestClass
on: #changes:scopes: respond: dialog;
on: #application: respond: dialog;
on: #refactoring: respond: dialog;
on: #openModal respond: dialog.
driver previewPresenterClass: requestClass.
]

{ #category : 'running' }
ReDuplicateClassDriverTest >> tearDown [

(testingEnvironment notNil and: [ testingEnvironment classNames includes: self classToBeDeleted ])
ifTrue: [ testingEnvironment removeClass: self classToBeDeleted ].
super tearDown.
]

{ #category : 'tests' }
ReDuplicateClassDriverTest >> testDuplicateClass [

| driver rbClass driverChanges |
| driver rbClass driverChanges refactoring methodChanges |

testingEnvironment := RBClassEnvironment class: self classToBeDuplicated.
driver := ReDuplicateClassDriver basicNew.

self setUpMocksOn: driver.

driver scopes: { testingEnvironment }.
rbClass := testingEnvironment class.
rbClass := testingEnvironment classes anyOne.

driver refactoring className: rbClass.
driver runRefactoring.
refactoring := ReCopyClassRefactoring new.
refactoring className: self classToBeDeleted.
refactoring sourceClass: rbClass.
driver runRefactoring: refactoring.
driverChanges := driver refactoring changes.
self
assert: driverChanges changes size
equals: 4.
self assertEmpty: driver refactoring failedApplicabilityPreconditions.
equals: 5.

methodChanges := driverChanges changes select: [ : amc | amc isKindOf: RBAddMethodChange ].
self
assert: (driverChanges changes select: [ : amc | amc selector = #hash or: [ amc selector = #= ] ]) size
assert: (methodChanges select: [ : mc | mc selector = #hash or: [ mc selector = #= ] ]) size
equals: 2.

testingEnvironment ifNotNil: [
testingEnvironment classesDo: [ : cls |
(cls includesSelector: #=)
ifTrue: [ cls removeSelector: #= ].
(cls includesSelector: #hash)
ifTrue: [ cls removeSelector: #hash ] ] ].

]
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ ReGenerateEqualAndHashDriverTest >> testGenerateEqualAndHashExistingImplementors
self assert: (driver targetClass definesMethod: #hash).
self assert: (driver targetClass definesMethod: #=).

driver selectDialog selectedItems: #(#iVarA #iVarB).
driver selectDialog selectedItems: #(#name #iVarB).
driver runRefactoring.
driverChanges := driver refactoring changes.

self
assert: driverChanges changes size
equals: 4.
equals: 2.
self assertEmpty: driver refactoring failedApplicabilityPreconditions.
self
assert: (driverChanges changes select: [ : amc | amc selector = #hash or: [ amc selector = #= ] ]) size
Expand All @@ -67,7 +67,7 @@ ReGenerateEqualAndHashDriverTest >> testGenerateEqualAndHashFirstGen [
driverChanges := driver refactoring changes.
self
assert: driverChanges changes size
equals: 4.
equals: 2.
self assertEmpty: driver refactoring failedApplicabilityPreconditions.
self
assert: (driverChanges changes select: [ : amc | amc selector = #hash or: [ amc selector = #= ] ]) size
Expand Down
11 changes: 11 additions & 0 deletions src/Refactoring-UI/ReDuplicateClassDriver.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,17 @@ ReDuplicateClassDriver >> runRefactoring [
self applyChanges
]

{ #category : 'execution' }
ReDuplicateClassDriver >> runRefactoring: aReCopyClassRefactoring [
"This is the same as #runRefactoring, but instead of automatically configuring a new empty refactoring we use aReCopyClassRefactoring"

refactoring := aReCopyClassRefactoring.
self requestNewClass.
requestDialog window isCancelled
ifTrue: [ ^ nil ].
self applyChanges
]

{ #category : 'accessing' }
ReDuplicateClassDriver >> scopes: refactoringScopes [

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ SycCmAddSubclassCommand >> newClassName: aString [

{ #category : 'accessing' }
SycCmAddSubclassCommand >> order [
^ 10
^ 10100.1
]

{ #category : 'accessing' }
Expand Down