The Trunk: Tests-mt.449.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Tests-mt.449.mcz

commits-2
Marcel Taeumel uploaded a new version of Tests to project The Trunk:
http://source.squeak.org/trunk/Tests-mt.449.mcz

==================== Summary ====================

Name: Tests-mt.449
Author: mt
Time: 13 April 2021, 10:51:15.223811 am
UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
Ancestors: Tests-nice.448

Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.

(Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)

=============== Diff against Tests-nice.448 ===============

Item was changed:
  TestCase subclass: #CompilerExceptionsTest
+ instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
- instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
  classVariableNames: ''
  poolDictionaries: ''
  category: 'Tests-Compiler'!

Item was added:
+ ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
+ generateUnknownSelector
+
+ | selector num |
+ selector := 'yourself'.
+ num := 0.
+ [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
+ ^ selector, num!

Item was removed:
- ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
- setUp
-
- super setUp.
- tearDowns := OrderedCollection new.
- Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
- tearDowns add: [Symbol intern: symbol]].
- Symbol extern: self unknownSelector.!

Item was changed:
  ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
  tearDown
 
  self removeGeneratedMethods.
- Symbol extern: self unknownSelector.
- tearDowns do: #value.
  super tearDown.!

Item was changed:
  ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
  testUnknownSelector
 
+ | unknownSelector |
+ self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
+ unknownSelector := self generateUnknownSelector.
  self
+ compiling: 'griffle self ' , unknownSelector
- compiling: 'griffle self ' , self unknownSelector
  shouldRaise: UnknownSelector
+ andSelect: unknownSelector
- andSelect: self unknownSelector
  testing: {
+ false -> [
+ self assertCanceled.
+ self assert: (Symbol lookup: unknownSelector) isNil].
+ 'yourself' -> [
+ self assertSucceeded: 'griffle self yourself'.
+ self assert: (Symbol lookup: unknownSelector) isNil].
+ unknownSelector -> [
- false -> [self assertCanceled].
- 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
- self unknownSelector -> [
  self assertSucceeded.
+ self assert: (Symbol lookup: unknownSelector) notNil] }.
+ unknownSelector := self generateUnknownSelector.
- self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
- Symbol extern: self unknownSelector] }.
  self
+ compiling: 'griffle [ self ' , unknownSelector , ' ] value'
- compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
  shouldRaise: UnknownSelector
+ andSelect: unknownSelector
- andSelect: self unknownSelector
  testing: {
+ false -> [
+ self assertCanceled.
+ self assert: (Symbol lookup: unknownSelector) isNil].
+ 'yourself' -> [
+ self assertSucceeded: 'griffle [ self yourself ] value'.
+ self assert: (Symbol lookup: unknownSelector) isNil].
+ unknownSelector -> [
- false -> [self assertCanceled].
- 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
- self unknownSelector -> [
  self assertSucceeded.
+ self assert: (Symbol lookup: unknownSelector) notNil] }.!
- self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!

Item was removed:
- ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
- unknownSelector
-
- ^ 'yourrsellff'!

Item was removed:
- ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
- extern: aStringOrSymbol
-
- {NewSymbols. SymbolTable} do: [:table |
- table remove: aStringOrSymbol ifAbsent: []].!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

Nicolas Cellier
Le mar. 13 avr. 2021 à 10:51, <[hidden email]> a écrit :

>
> Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> http://source.squeak.org/trunk/Tests-mt.449.mcz
>
> ==================== Summary ====================
>
> Name: Tests-mt.449
> Author: mt
> Time: 13 April 2021, 10:51:15.223811 am
> UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> Ancestors: Tests-nice.448
>
> Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
>
> (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)

May I suggest to classify them as private then?

>
> =============== Diff against Tests-nice.448 ===============
>
> Item was changed:
>   TestCase subclass: #CompilerExceptionsTest
> +       instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> -       instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
>         classVariableNames: ''
>         poolDictionaries: ''
>         category: 'Tests-Compiler'!
>
> Item was added:
> + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> + generateUnknownSelector
> +
> +       | selector num |
> +       selector := 'yourself'.
> +       num := 0.
> +       [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> +       ^ selector, num!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> - setUp
> -
> -       super setUp.
> -       tearDowns := OrderedCollection new.
> -       Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> -               tearDowns add: [Symbol intern: symbol]].
> -       Symbol extern: self unknownSelector.!
>
> Item was changed:
>   ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
>   tearDown
>
>         self removeGeneratedMethods.
> -       Symbol extern: self unknownSelector.
> -       tearDowns do: #value.
>         super tearDown.!
>
> Item was changed:
>   ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
>   testUnknownSelector
>
> +       | unknownSelector |
> +       self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> +       unknownSelector := self generateUnknownSelector.
>         self
> +               compiling: 'griffle self ' , unknownSelector
> -               compiling: 'griffle self ' , self unknownSelector
>                 shouldRaise: UnknownSelector
> +               andSelect: unknownSelector
> -               andSelect: self unknownSelector
>                 testing: {
> +                       false -> [
> +                               self assertCanceled.
> +                               self assert: (Symbol lookup: unknownSelector) isNil].
> +                       'yourself' -> [
> +                               self assertSucceeded: 'griffle self yourself'.
> +                               self assert: (Symbol lookup: unknownSelector) isNil].
> +                       unknownSelector -> [
> -                       false -> [self assertCanceled].
> -                       'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> -                       self unknownSelector -> [
>                                 self assertSucceeded.
> +                               self assert: (Symbol lookup: unknownSelector) notNil] }.
> +       unknownSelector := self generateUnknownSelector.
> -                               self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> -                               Symbol extern: self unknownSelector] }.
>         self
> +               compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> -               compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
>                 shouldRaise: UnknownSelector
> +               andSelect: unknownSelector
> -               andSelect: self unknownSelector
>                 testing: {
> +                       false -> [
> +                               self assertCanceled.
> +                               self assert: (Symbol lookup: unknownSelector) isNil].
> +                       'yourself' -> [
> +                               self assertSucceeded: 'griffle [ self yourself ] value'.
> +                               self assert: (Symbol lookup: unknownSelector) isNil].
> +                       unknownSelector -> [
> -                       false -> [self assertCanceled].
> -                       'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> -                       self unknownSelector -> [
>                                 self assertSucceeded.
> +                               self assert: (Symbol lookup: unknownSelector) notNil] }.!
> -                               self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> - unknownSelector
> -
> -       ^ 'yourrsellff'!
>
> Item was removed:
> - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> - extern: aStringOrSymbol
> -
> -       {NewSymbols. SymbolTable} do: [:table |
> -               table remove: aStringOrSymbol ifAbsent: []].!
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

marcel.taeumel
May I suggest to classify them as private then?

Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:. 

Best,
Marcel

Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>:

Le mar. 13 avr. 2021 à 10:51, a écrit :
>
> Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> http://source.squeak.org/trunk/Tests-mt.449.mcz
>
> ==================== Summary ====================
>
> Name: Tests-mt.449
> Author: mt
> Time: 13 April 2021, 10:51:15.223811 am
> UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> Ancestors: Tests-nice.448
>
> Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
>
> (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)

May I suggest to classify them as private then?

>
> =============== Diff against Tests-nice.448 ===============
>
> Item was changed:
> TestCase subclass: #CompilerExceptionsTest
> + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
> classVariableNames: ''
> poolDictionaries: ''
> category: 'Tests-Compiler'!
>
> Item was added:
> + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> + generateUnknownSelector
> +
> + | selector num |
> + selector := 'yourself'.
> + num := 0.
> + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> + ^ selector, num!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> - setUp
> -
> - super setUp.
> - tearDowns := OrderedCollection new.
> - Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> - tearDowns add: [Symbol intern: symbol]].
> - Symbol extern: self unknownSelector.!
>
> Item was changed:
> ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
> tearDown
>
> self removeGeneratedMethods.
> - Symbol extern: self unknownSelector.
> - tearDowns do: #value.
> super tearDown.!
>
> Item was changed:
> ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
> testUnknownSelector
>
> + | unknownSelector |
> + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> + unknownSelector := self generateUnknownSelector.
> self
> + compiling: 'griffle self ' , unknownSelector
> - compiling: 'griffle self ' , self unknownSelector
> shouldRaise: UnknownSelector
> + andSelect: unknownSelector
> - andSelect: self unknownSelector
> testing: {
> + false -> [
> + self assertCanceled.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + 'yourself' -> [
> + self assertSucceeded: 'griffle self yourself'.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + unknownSelector -> [
> - false -> [self assertCanceled].
> - 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> - self unknownSelector -> [
> self assertSucceeded.
> + self assert: (Symbol lookup: unknownSelector) notNil] }.
> + unknownSelector := self generateUnknownSelector.
> - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> - Symbol extern: self unknownSelector] }.
> self
> + compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> - compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
> shouldRaise: UnknownSelector
> + andSelect: unknownSelector
> - andSelect: self unknownSelector
> testing: {
> + false -> [
> + self assertCanceled.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + 'yourself' -> [
> + self assertSucceeded: 'griffle [ self yourself ] value'.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + unknownSelector -> [
> - false -> [self assertCanceled].
> - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> - self unknownSelector -> [
> self assertSucceeded.
> + self assert: (Symbol lookup: unknownSelector) notNil] }.!
> - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> - unknownSelector
> -
> - ^ 'yourrsellff'!
>
> Item was removed:
> - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> - extern: aStringOrSymbol
> -
> - {NewSymbols. SymbolTable} do: [:table |
> - table remove: aStringOrSymbol ifAbsent: []].!
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

marcel.taeumel
Hey Eliot,

recently, you chose #findInterned: over #lookup:. Can you recall how this happened? What is your opinion on this matter? :-)

Here is a remark from Levente about this from July 2019:

Best,
Marcel

Am 13.04.2021 11:44:24 schrieb Marcel Taeumel <[hidden email]>:

May I suggest to classify them as private then?

Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:. 

Best,
Marcel

Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>:

Le mar. 13 avr. 2021 à 10:51, a écrit :

>
> Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> http://source.squeak.org/trunk/Tests-mt.449.mcz
>
> ==================== Summary ====================
>
> Name: Tests-mt.449
> Author: mt
> Time: 13 April 2021, 10:51:15.223811 am
> UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> Ancestors: Tests-nice.448
>
> Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
>
> (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)

May I suggest to classify them as private then?

>
> =============== Diff against Tests-nice.448 ===============
>
> Item was changed:
> TestCase subclass: #CompilerExceptionsTest
> + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
> classVariableNames: ''
> poolDictionaries: ''
> category: 'Tests-Compiler'!
>
> Item was added:
> + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> + generateUnknownSelector
> +
> + | selector num |
> + selector := 'yourself'.
> + num := 0.
> + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> + ^ selector, num!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> - setUp
> -
> - super setUp.
> - tearDowns := OrderedCollection new.
> - Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> - tearDowns add: [Symbol intern: symbol]].
> - Symbol extern: self unknownSelector.!
>
> Item was changed:
> ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
> tearDown
>
> self removeGeneratedMethods.
> - Symbol extern: self unknownSelector.
> - tearDowns do: #value.
> super tearDown.!
>
> Item was changed:
> ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
> testUnknownSelector
>
> + | unknownSelector |
> + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> + unknownSelector := self generateUnknownSelector.
> self
> + compiling: 'griffle self ' , unknownSelector
> - compiling: 'griffle self ' , self unknownSelector
> shouldRaise: UnknownSelector
> + andSelect: unknownSelector
> - andSelect: self unknownSelector
> testing: {
> + false -> [
> + self assertCanceled.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + 'yourself' -> [
> + self assertSucceeded: 'griffle self yourself'.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + unknownSelector -> [
> - false -> [self assertCanceled].
> - 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> - self unknownSelector -> [
> self assertSucceeded.
> + self assert: (Symbol lookup: unknownSelector) notNil] }.
> + unknownSelector := self generateUnknownSelector.
> - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> - Symbol extern: self unknownSelector] }.
> self
> + compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> - compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
> shouldRaise: UnknownSelector
> + andSelect: unknownSelector
> - andSelect: self unknownSelector
> testing: {
> + false -> [
> + self assertCanceled.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + 'yourself' -> [
> + self assertSucceeded: 'griffle [ self yourself ] value'.
> + self assert: (Symbol lookup: unknownSelector) isNil].
> + unknownSelector -> [
> - false -> [self assertCanceled].
> - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> - self unknownSelector -> [
> self assertSucceeded.
> + self assert: (Symbol lookup: unknownSelector) notNil] }.!
> - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
>
> Item was removed:
> - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> - unknownSelector
> -
> - ^ 'yourrsellff'!
>
> Item was removed:
> - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> - extern: aStringOrSymbol
> -
> - {NewSymbols. SymbolTable} do: [:table |
> - table remove: aStringOrSymbol ifAbsent: []].!
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

Nicolas Cellier
In reply to this post by marcel.taeumel
findInterned: could be deprecated because completely equivalent to lookup:.
I removed its usage in core image, but there are other in external
packages (refactoring browser for example).

Le mar. 13 avr. 2021 à 11:44, Marcel Taeumel <[hidden email]> a écrit :

>
> > May I suggest to classify them as private then?
>
> Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:.
>
> Best,
> Marcel
>
> Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>:
>
> Le mar. 13 avr. 2021 à 10:51, a écrit :
> >
> > Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> > http://source.squeak.org/trunk/Tests-mt.449.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Tests-mt.449
> > Author: mt
> > Time: 13 April 2021, 10:51:15.223811 am
> > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> > Ancestors: Tests-nice.448
> >
> > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
> >
> > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)
>
> May I suggest to classify them as private then?
>
> >
> > =============== Diff against Tests-nice.448 ===============
> >
> > Item was changed:
> > TestCase subclass: #CompilerExceptionsTest
> > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
> > classVariableNames: ''
> > poolDictionaries: ''
> > category: 'Tests-Compiler'!
> >
> > Item was added:
> > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> > + generateUnknownSelector
> > +
> > + | selector num |
> > + selector := 'yourself'.
> > + num := 0.
> > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> > + ^ selector, num!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> > - setUp
> > -
> > - super setUp.
> > - tearDowns := OrderedCollection new.
> > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> > - tearDowns add: [Symbol intern: symbol]].
> > - Symbol extern: self unknownSelector.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
> > tearDown
> >
> > self removeGeneratedMethods.
> > - Symbol extern: self unknownSelector.
> > - tearDowns do: #value.
> > super tearDown.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
> > testUnknownSelector
> >
> > + | unknownSelector |
> > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> > + unknownSelector := self generateUnknownSelector.
> > self
> > + compiling: 'griffle self ' , unknownSelector
> > - compiling: 'griffle self ' , self unknownSelector
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle self yourself'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.
> > + unknownSelector := self generateUnknownSelector.
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> > - Symbol extern: self unknownSelector] }.
> > self
> > + compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle [ self yourself ] value'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.!
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> > - unknownSelector
> > -
> > - ^ 'yourrsellff'!
> >
> > Item was removed:
> > - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> > - extern: aStringOrSymbol
> > -
> > - {NewSymbols. SymbolTable} do: [:table |
> > - table remove: aStringOrSymbol ifAbsent: []].!
> >
> >
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

Nicolas Cellier
In reply to this post by marcel.taeumel
I think that findInterned: is old, probably from st80 epoch...
hasInterned:ifTrue: also, it may be cross-dialect. Strange that we did
put it in private...
a massive rewrite of (Symbol hasInterned: aString ifTrue: [:aSymbol |
...]) into ((Symbol lookup: aString) ifNotNil: [:aSymbol | ...]) is
possible, except in the rare cases when we use the returned value of
the send, which is a Boolean.
But it is questionable...

Le mar. 13 avr. 2021 à 11:53, Marcel Taeumel <[hidden email]> a écrit :

>
> Hey Eliot,
>
> recently, you chose #findInterned: over #lookup:. Can you recall how this happened? What is your opinion on this matter? :-)
>
> Here is a remark from Levente about this from July 2019:
> http://forum.world.st/The-Trunk-Morphic-mt-1491-mcz-tp5101075p5101089.html
>
> Best,
> Marcel
>
> Am 13.04.2021 11:44:24 schrieb Marcel Taeumel <[hidden email]>:
>
> > May I suggest to classify them as private then?
>
> Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:.
>
> Best,
> Marcel
>
> Am 13.04.2021 11:25:27 schrieb Nicolas Cellier <[hidden email]>:
>
> Le mar. 13 avr. 2021 à 10:51, a écrit :
> >
> > Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> > http://source.squeak.org/trunk/Tests-mt.449.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Tests-mt.449
> > Author: mt
> > Time: 13 April 2021, 10:51:15.223811 am
> > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> > Ancestors: Tests-nice.448
> >
> > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
> >
> > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)
>
> May I suggest to classify them as private then?
>
> >
> > =============== Diff against Tests-nice.448 ===============
> >
> > Item was changed:
> > TestCase subclass: #CompilerExceptionsTest
> > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
> > classVariableNames: ''
> > poolDictionaries: ''
> > category: 'Tests-Compiler'!
> >
> > Item was added:
> > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> > + generateUnknownSelector
> > +
> > + | selector num |
> > + selector := 'yourself'.
> > + num := 0.
> > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> > + ^ selector, num!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> > - setUp
> > -
> > - super setUp.
> > - tearDowns := OrderedCollection new.
> > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> > - tearDowns add: [Symbol intern: symbol]].
> > - Symbol extern: self unknownSelector.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
> > tearDown
> >
> > self removeGeneratedMethods.
> > - Symbol extern: self unknownSelector.
> > - tearDowns do: #value.
> > super tearDown.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
> > testUnknownSelector
> >
> > + | unknownSelector |
> > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> > + unknownSelector := self generateUnknownSelector.
> > self
> > + compiling: 'griffle self ' , unknownSelector
> > - compiling: 'griffle self ' , self unknownSelector
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle self yourself'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.
> > + unknownSelector := self generateUnknownSelector.
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> > - Symbol extern: self unknownSelector] }.
> > self
> > + compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle [ self yourself ] value'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.!
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> > - unknownSelector
> > -
> > - ^ 'yourrsellff'!
> >
> > Item was removed:
> > - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> > - extern: aStringOrSymbol
> > -
> > - {NewSymbols. SymbolTable} do: [:table |
> > - table remove: aStringOrSymbol ifAbsent: []].!
> >
> >
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tests-mt.449.mcz

marcel.taeumel
a massive rewrite of (Symbol hasInterned: aString ifTrue: [:aSymbol |
...]) into ((Symbol lookup: aString) ifNotNil: [:aSymbol | ...]) is
possible

Hmm... only 34 senders. Would me take about 2 hours max, I suppose. I volunteer. Should I do it?

Best,
Marcel

Am 13.04.2021 12:01:05 schrieb Nicolas Cellier <[hidden email]>:

I think that findInterned: is old, probably from st80 epoch...
hasInterned:ifTrue: also, it may be cross-dialect. Strange that we did
put it in private...
a massive rewrite of (Symbol hasInterned: aString ifTrue: [:aSymbol |
...]) into ((Symbol lookup: aString) ifNotNil: [:aSymbol | ...]) is
possible, except in the rare cases when we use the returned value of
the send, which is a Boolean.
But it is questionable...

Le mar. 13 avr. 2021 à 11:53, Marcel Taeumel a écrit :
>
> Hey Eliot,
>
> recently, you chose #findInterned: over #lookup:. Can you recall how this happened? What is your opinion on this matter? :-)
>
> Here is a remark from Levente about this from July 2019:
> http://forum.world.st/The-Trunk-Morphic-mt-1491-mcz-tp5101075p5101089.html
>
> Best,
> Marcel
>
> Am 13.04.2021 11:44:24 schrieb Marcel Taeumel :
>
> > May I suggest to classify them as private then?
>
> Hmm... #hasInterned:ifTrue: is "private" already. ;-) Maybe #findInterned: should be, too. Yet, soft-deprecation via "self flag: #deprecated" might be even better since both are actually implemented through #lookup:.
>
> Best,
> Marcel
>
> Am 13.04.2021 11:25:27 schrieb Nicolas Cellier :
>
> Le mar. 13 avr. 2021 à 10:51, a écrit :
> >
> > Marcel Taeumel uploaded a new version of Tests to project The Trunk:
> > http://source.squeak.org/trunk/Tests-mt.449.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Tests-mt.449
> > Author: mt
> > Time: 13 April 2021, 10:51:15.223811 am
> > UUID: 23984a3a-fc1c-2a44-a023-9862b3d2ba8d
> > Ancestors: Tests-nice.448
> >
> > Simplify #testUnknownSelector. Avoid interfering with garbage collection and symbol tables.
> >
> > (Note that #lookup: is the preferred interface, not #findInterned: or #hasInterned:ifTrue:. The concept of internalization should not leak into client code.)
>
> May I suggest to classify them as private then?
>
> >
> > =============== Diff against Tests-nice.448 ===============
> >
> > Item was changed:
> > TestCase subclass: #CompilerExceptionsTest
> > + instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection'
> > - instanceVariableNames: 'text selectionInterval originalText previousSelection originalSelection tearDowns'
> > classVariableNames: ''
> > poolDictionaries: ''
> > category: 'Tests-Compiler'!
> >
> > Item was added:
> > + ----- Method: CompilerExceptionsTest>>generateUnknownSelector (in category 'private') -----
> > + generateUnknownSelector
> > +
> > + | selector num |
> > + selector := 'yourself'.
> > + num := 0.
> > + [(Symbol lookup: selector, num) notNil] whileTrue: [num := num + 1].
> > + ^ selector, num!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>setUp (in category 'running') -----
> > - setUp
> > -
> > - super setUp.
> > - tearDowns := OrderedCollection new.
> > - Symbol hasInterned: self unknownSelector ifTrue: [:symbol |
> > - tearDowns add: [Symbol intern: symbol]].
> > - Symbol extern: self unknownSelector.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>tearDown (in category 'running') -----
> > tearDown
> >
> > self removeGeneratedMethods.
> > - Symbol extern: self unknownSelector.
> > - tearDowns do: #value.
> > super tearDown.!
> >
> > Item was changed:
> > ----- Method: CompilerExceptionsTest>>testUnknownSelector (in category 'tests') -----
> > testUnknownSelector
> >
> > + | unknownSelector |
> > + self flag: #flaky. "mt: This test depends on a list of suggestions that is derived from the system's known selectors. Any desired replacement might not be present in the actual list. Maybe we could use an index-based choice instead."
> > + unknownSelector := self generateUnknownSelector.
> > self
> > + compiling: 'griffle self ' , unknownSelector
> > - compiling: 'griffle self ' , self unknownSelector
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle self yourself'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle self yourself'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.
> > + unknownSelector := self generateUnknownSelector.
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |]).
> > - Symbol extern: self unknownSelector] }.
> > self
> > + compiling: 'griffle [ self ' , unknownSelector , ' ] value'
> > - compiling: 'griffle [ self ' , self unknownSelector , ' ] value'
> > shouldRaise: UnknownSelector
> > + andSelect: unknownSelector
> > - andSelect: self unknownSelector
> > testing: {
> > + false -> [
> > + self assertCanceled.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + 'yourself' -> [
> > + self assertSucceeded: 'griffle [ self yourself ] value'.
> > + self assert: (Symbol lookup: unknownSelector) isNil].
> > + unknownSelector -> [
> > - false -> [self assertCanceled].
> > - 'yourself' -> [self assertSucceeded: 'griffle [ self yourself ] value'].
> > - self unknownSelector -> [
> > self assertSucceeded.
> > + self assert: (Symbol lookup: unknownSelector) notNil] }.!
> > - self assert: (Symbol hasInterned: self unknownSelector ifTrue: [:symbol |])] }.!
> >
> > Item was removed:
> > - ----- Method: CompilerExceptionsTest>>unknownSelector (in category 'private') -----
> > - unknownSelector
> > -
> > - ^ 'yourrsellff'!
> >
> > Item was removed:
> > - ----- Method: Symbol class>>extern: (in category '*Tests-release') -----
> > - extern: aStringOrSymbol
> > -
> > - {NewSymbols. SymbolTable} do: [:table |
> > - table remove: aStringOrSymbol ifAbsent: []].!
> >
> >
>
>