Nicolai Hess wrote:
Hope we can sort this out, more and more fixes suffer from the wrong testing
failures and nothing new gets integrated.


12996 ClassTest>>#testRenaming leaves dirty package Dummy-Tests-Class behind
12995 CI ClassTest>>#testRenaming is failing

available options:
1. skip test
2. test for nil in RPackageOrganizer>>#systemClassRenamedActionFrom:
   before
   rPackage updateDefinedClassNamed: oldName withNewName: newName.
3. review changes for issue 12601
    (fix RPackageOrganizer>>#systemCategoryRemovedActionFrom: ann, to
    unregister packages even if there are MCPackages (don't know about the
    side effects)

best solution would be number 3, but the recent submissions for
issue 12601 are quite complex, so I don't know.

Nicolai
Here is something I was developing last week for
    Case 12935 ManifestTests-leaving-dirty-packages
but I got distracted so its working but incomplete. It requires a higher level architectural review as to whether this is a worthwhile approach.

I had trouble understanding how to undo the state of the the dirty package being left after test resources in the package were modified, so following the engineering principle "if you can't solve the problem, change the problem," I thought creating and deleting a temporary package to hold the test resources would be less impact than dirtying the existing packages. I also knew I could dig into the logic behind the Monticello button <+Package> and package context menu item [Unload package] to copy a working system.  So far I have the following (also attached as ST files)....


"-------"
Object subclass: #TemporaryTestResourceBuilder
    instanceVariableNames: ''
    classVariableNames: ''
    category: 'Manifest-Tests'
"-------"

TemporaryTestResourceBuilder class
    instanceVariableNames: 'workingCopy'
"-------"

TemporaryTestResourceBuilder>>temporaryTestPackageName
    ^ 'ZZZZ-Temporary-Test-Resources-Manifest-Tests'.
"-------"

TemporaryTestResourceBuilder>>defineTemporaryPackage
    RPackageOrganizer default registerPackageNamed: self packageName.
    workingCopy := MCWorkingCopy forPackage: (MCPackage new name: self packageName).
"-------"

TemporaryTestResourceBuilder>>unloadTemporaryPackage
    workingCopy unload.
"-------"

TemporaryTestResourceBuilder>>defineTemporaryMFClassA
    "Define the class"
    Object subclass: #MFClassA
    instanceVariableNames: ''
    classVariableNames: ''
    category: self temporaryTestPackageName.
   
    (Smalltalk at: #MFClassA) compileSilently: 'method
    |foo|
    self halt.
    '.
       
    ^Smalltalk at: #MFClassA.
"-------"

defineTemporaryMFClassB
    "Define the class"
    Object subclass: #MFClassB
    instanceVariableNames: ''
    classVariableNames: ''
    category: self temporaryTestPackageName.
   
    (Smalltalk at: #MFClassB) compileSilently: 'method2
    |foo|
    '.
   
    (Smalltalk at: #MFClassB) compileSilently: 'method3
    |foo|
    self halt.
    '.
     
    ^Smalltalk at: #MFClassB.
"------------------------------------------------------------------"

TestCase subclass: #SmalllintManifestCheckerTest
    instanceVariableNames: 'checker mfClassA mfClassB'
    classVariableNames: ''
    category: 'Manifest-Tests'
"-------"

SmalllintManifestCheckerTest>>tearDown
    TemporaryTestResourceBuilder unloadTemporaryPackage.
"-------"

SmalllintManifestCheckerTest>>setUp
    | bm |
    TemporaryTestResourceBuilder defineTemporaryPackage.
    mfClassA := TemporaryTestResourceBuilder defineTemporaryMFClassA.
    mfClassB := TemporaryTestResourceBuilder defineTemporaryMFClassB.

    "BTC: The following is unchanged except converting direct class references to the instance variables"
    bm := TheManifestBuilder of: mfClassA.
    bm installFalsePositiveOf: RBCodeCruftLeftInMethodsRule uniqueIdentifierName version: 1.
    bm addFalsePositive: mfClassB >> #method3 of: RBCodeCruftLeftInMethodsRule uniqueIdentifierName version: 1.
    bm installToDoOf: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName version: 1.
    bm
        addAllToDo:
            {(mfClassB >> #method3).
            (mfClassA >> #method)}
        of: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName
        version: 1.
    checker := SmalllintManifestChecker new
"-------"

SmalllintManifestCheckerTest>>testIsFalsePositive
    "BTC: No changes here except converting direct class references to instance variables"
    | rule |
    rule  := RBCompositeLintRule allGoodRules.
    checker
        rule: rule;
        environment: self package asEnvironment;
        run. 
    self assert: (checker isFalsePositive:  (mfClassB>>#method3) forRuleId: (RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).
    self deny: (checker isFalsePositive:  (mfClassA>>#method) forRuleId: (RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).
"------------------------------------------------------------------"

Now the following test executed from Workspace works fine...
    SmalllintManifestCheckerTest new setUp testIsFalsePositive tearDown.
"------------------------------------------------------------------"

Some reflection on this:
* TemporaryTestResourceBuilder class-side stuff could move to instance-side, so that SmalllintManifestCheckerTest can store an instance of it, such that when instance is being garbage collected the finalisation might ensure the temporary package is unloaded.
* Rather than store the classes in instance variables to be used in the form (mfClassB >> method3), a better form might be (#MFClassB asClass >> method3).

cheers -ben
'From Pharo3.0 of 18 March 2013 [Latest update: #30778] on 1 March 2014 at 
10:10:40.919902 am'!
Object subclass: #TemporaryTestResourceBuilder
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Manifest-Tests'!
!TemporaryTestResourceBuilder commentStamp: '<historical>' prior: 0!
!


"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

TemporaryTestResourceBuilder class
        instanceVariableNames: 'workingCopy'!
!TemporaryTestResourceBuilder class commentStamp: '<historical>' prior: 0!
!


!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:27'!
temporaryTestPackageName
        ^ 'ZZZZ-Dynamic-Manifest-Tests-Resources'.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:28'!
defineTemporaryPackage
        RPackageOrganizer default registerPackageNamed: self 
temporaryTestPackageName.
        workingCopy := MCWorkingCopy forPackage: (MCPackage new name: self 
temporaryTestPackageName).
! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 10:01'!
defineTemporaryMFClassA
        "Define the class"
        Object subclass: #MFClassA
        instanceVariableNames: ''
        classVariableNames: ''
        category: self temporaryTestPackageName.
        
        (Smalltalk at: #MFClassA) compileSilently: 'method
        |foo|
        self halt.
        '.
        
        ^Smalltalk at: #MFClassA.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 2/24/2014 01:46'!
unloadTemporaryPackage
        workingCopy unload.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:26'!
testPackageName
        ^ 'ZZZZ-Dynamic-Manifest-Tests-Resources'.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 10:01'!
defineTemporaryMFClassB
        "Define the class"
        Object subclass: #MFClassB
        instanceVariableNames: ''
        classVariableNames: ''
        category: self temporaryTestPackageName.
        
        (Smalltalk at: #MFClassB) compileSilently: 'method2
        |foo|
        '.
        
        (Smalltalk at: #MFClassB) compileSilently: 'method3
        |foo|
        self halt.
        '.
                
        ^Smalltalk at: #MFClassB.! !


!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:27'!
temporaryTestPackageName
        ^ 'ZZZZ-Dynamic-Manifest-Tests-Resources'.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:28'!
defineTemporaryPackage
        RPackageOrganizer default registerPackageNamed: self 
temporaryTestPackageName.
        workingCopy := MCWorkingCopy forPackage: (MCPackage new name: self 
temporaryTestPackageName).
! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 10:01'!
defineTemporaryMFClassA
        "Define the class"
        Object subclass: #MFClassA
        instanceVariableNames: ''
        classVariableNames: ''
        category: self temporaryTestPackageName.
        
        (Smalltalk at: #MFClassA) compileSilently: 'method
        |foo|
        self halt.
        '.
        
        ^Smalltalk at: #MFClassA.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 2/24/2014 01:46'!
unloadTemporaryPackage
        workingCopy unload.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 09:26'!
testPackageName
        ^ 'ZZZZ-Dynamic-Manifest-Tests-Resources'.! !

!TemporaryTestResourceBuilder class methodsFor: 'as yet unclassified' stamp: 
'BenComan 3/1/2014 10:01'!
defineTemporaryMFClassB
        "Define the class"
        Object subclass: #MFClassB
        instanceVariableNames: ''
        classVariableNames: ''
        category: self temporaryTestPackageName.
        
        (Smalltalk at: #MFClassB) compileSilently: 'method2
        |foo|
        '.
        
        (Smalltalk at: #MFClassB) compileSilently: 'method3
        |foo|
        self halt.
        '.
                
        ^Smalltalk at: #MFClassB.! !
'From Pharo3.0 of 18 March 2013 [Latest update: #30778] on 1 March 2014 at 
10:10:35.531902 am'!
TestCase subclass: #SmalllintManifestCheckerTest
        instanceVariableNames: 'checker mfClassA mfClassB'
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Manifest-Tests'!
!SmalllintManifestCheckerTest commentStamp: 'TorstenBergmann 2/19/2014 08:34' 
prior: 0!
SUnit tests for SmalllintManifestChecker!


!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:45'!
testToDoOf

        | rule |
        rule := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.

        self assert: (( checker toDoOf: RBOnlyReadOrWrittenTemporaryRule new) 
anySatisfy: [:each| each = (mfClassB>>#method3)]).
        self deny: (( checker toDoOf: RBOnlyReadOrWrittenTemporaryRule new) 
anySatisfy: [:each| each = (mfClassB>>#method2)]).! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:56'!
testIsToDo

        | rule |
        rule  := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.
        
        self assert: (checker isToDo:  (mfClassB>>#method3) forRuleId: 
(RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName) versionId:  1).
        self deny: (checker isToDo:  (mfClassB>>#method2) forRuleId: 
(RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName) versionId:  1).

! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:57'!
testCriticsOf

        | rule |
        rule := RBCompositeLintRule allGoodRules.
        checker 
                rule: rule;
                environment: self package asEnvironment;
                run. 
        
        self assert: (checker criticsOf: RBOnlyReadOrWrittenTemporaryRule new) 
size  = 3.
        self assert: (( checker criticsOf: RBOnlyReadOrWrittenTemporaryRule new 
) anySatisfy: [:each| each = (mfClassB>>#method3)]).
        self assert: (( checker criticsOf: RBOnlyReadOrWrittenTemporaryRule 
new) anySatisfy: [:each| each = (mfClassA>>#method)]).! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:56'!
testIsFalsePositive 

        | rule |
        rule  := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.  
        self assert: (checker isFalsePositive:  (mfClassB>>#method3) forRuleId: 
(RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).
        self deny: (checker isFalsePositive:  (mfClassA>>#method) forRuleId: 
(RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).

! !

!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'StephaneDucasse 
3/21/2013 13:51'!
cleaningResources
        Smalltalk globals
                at: #ManifestManifestResourcesTests
                ifPresent: [ :cl | 
                        cl
                                removeFromChanges;
                                removeFromSystemUnlogged ]! !

!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'BenComan 3/1/2014 
10:02'!
setUp
        | bm |
"       self cleaningResources.
"       TemporaryTestResourceBuilder defineTemporaryPackage.
        mfClassA := TemporaryTestResourceBuilder defineTemporaryMFClassA.
        mfClassB := TemporaryTestResourceBuilder defineTemporaryMFClassB.
        
        bm := TheManifestBuilder of: mfClassA.
        bm installFalsePositiveOf: RBCodeCruftLeftInMethodsRule 
uniqueIdentifierName version: 1.
        bm addFalsePositive: mfClassB >> #method3 of: 
RBCodeCruftLeftInMethodsRule uniqueIdentifierName version: 1.
        bm installToDoOf: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName 
version: 1.
        bm
                addAllToDo:
                        {(mfClassB >> #method3).
                        (mfClassA >> #method)}
                of: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName
                version: 1.
        checker := SmalllintManifestChecker new! !

!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'BenComan 3/1/2014 
09:51'!
tearDown
        
        TemporaryTestResourceBuilder unloadTemporaryPackage.! !

!SmalllintManifestCheckerTest methodsFor: 'private' stamp: 'BenComan 2/24/2014 
01:59'!
package 
        MCWorkingCopy managersForClass: mfClassA  do: [:p | ^ p packageSet 
packages first].
        " should be equivalent to RPackageOrganizer default packageNamed: 
#'Manifest-Resources-Tests' "! !


!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:45'!
testToDoOf

        | rule |
        rule := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.

        self assert: (( checker toDoOf: RBOnlyReadOrWrittenTemporaryRule new) 
anySatisfy: [:each| each = (mfClassB>>#method3)]).
        self deny: (( checker toDoOf: RBOnlyReadOrWrittenTemporaryRule new) 
anySatisfy: [:each| each = (mfClassB>>#method2)]).! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:56'!
testIsToDo

        | rule |
        rule  := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.
        
        self assert: (checker isToDo:  (mfClassB>>#method3) forRuleId: 
(RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName) versionId:  1).
        self deny: (checker isToDo:  (mfClassB>>#method2) forRuleId: 
(RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName) versionId:  1).

! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:57'!
testCriticsOf

        | rule |
        rule := RBCompositeLintRule allGoodRules.
        checker 
                rule: rule;
                environment: self package asEnvironment;
                run. 
        
        self assert: (checker criticsOf: RBOnlyReadOrWrittenTemporaryRule new) 
size  = 3.
        self assert: (( checker criticsOf: RBOnlyReadOrWrittenTemporaryRule new 
) anySatisfy: [:each| each = (mfClassB>>#method3)]).
        self assert: (( checker criticsOf: RBOnlyReadOrWrittenTemporaryRule 
new) anySatisfy: [:each| each = (mfClassA>>#method)]).! !

!SmalllintManifestCheckerTest methodsFor: 'tests' stamp: 'BenComan 2/24/2014 
01:56'!
testIsFalsePositive 

        | rule |
        rule  := RBCompositeLintRule allGoodRules.
        checker
                rule: rule;
                environment: self package asEnvironment;
                run.  
        self assert: (checker isFalsePositive:  (mfClassB>>#method3) forRuleId: 
(RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).
        self deny: (checker isFalsePositive:  (mfClassA>>#method) forRuleId: 
(RBCodeCruftLeftInMethodsRule uniqueIdentifierName) versionId:  1).

! !


!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'StephaneDucasse 
3/21/2013 13:51'!
cleaningResources
        Smalltalk globals
                at: #ManifestManifestResourcesTests
                ifPresent: [ :cl | 
                        cl
                                removeFromChanges;
                                removeFromSystemUnlogged ]! !

!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'BenComan 3/1/2014 
10:02'!
setUp
        | bm |
"       self cleaningResources.
"       TemporaryTestResourceBuilder defineTemporaryPackage.
        mfClassA := TemporaryTestResourceBuilder defineTemporaryMFClassA.
        mfClassB := TemporaryTestResourceBuilder defineTemporaryMFClassB.
        
        bm := TheManifestBuilder of: mfClassA.
        bm installFalsePositiveOf: RBCodeCruftLeftInMethodsRule 
uniqueIdentifierName version: 1.
        bm addFalsePositive: mfClassB >> #method3 of: 
RBCodeCruftLeftInMethodsRule uniqueIdentifierName version: 1.
        bm installToDoOf: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName 
version: 1.
        bm
                addAllToDo:
                        {(mfClassB >> #method3).
                        (mfClassA >> #method)}
                of: RBOnlyReadOrWrittenTemporaryRule uniqueIdentifierName
                version: 1.
        checker := SmalllintManifestChecker new! !

!SmalllintManifestCheckerTest methodsFor: 'running' stamp: 'BenComan 3/1/2014 
09:51'!
tearDown
        
        TemporaryTestResourceBuilder unloadTemporaryPackage.! !


!SmalllintManifestCheckerTest methodsFor: 'private' stamp: 'BenComan 2/24/2014 
01:59'!
package 
        MCWorkingCopy managersForClass: mfClassA  do: [:p | ^ p packageSet 
packages first].
        " should be equivalent to RPackageOrganizer default packageNamed: 
#'Manifest-Resources-Tests' "! !

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

SmalllintManifestCheckerTest class
        instanceVariableNames: 'mfClassA'!
!SmalllintManifestCheckerTest class commentStamp: '<historical>' prior: 0!
!


!SmalllintManifestCheckerTest class methodsFor: 'as yet unclassified' stamp: 
'BenComan 2/24/2014 01:59'!
DoIt
        ^ MCWorkingCopy
                managersForClass: mfClassA
                do: [:p | ^ p first] ! !


!SmalllintManifestCheckerTest class methodsFor: 'as yet unclassified' stamp: 
'BenComan 2/24/2014 01:59'!
DoIt
        ^ MCWorkingCopy
                managersForClass: mfClassA
                do: [:p | ^ p first] ! !

Reply via email to