[squeak-dev] The Inbox: Tests-cwp.281.mcz

Frank Shearar frank.shearar at gmail.com
Wed Jan 1 21:31:41 UTC 2014


On 1 January 2014 18:41,  <commits at source.squeak.org> wrote:
> A new version of Tests was added to project The Inbox:
> http://source.squeak.org/inbox/Tests-cwp.281.mcz
>
> ==================== Summary ====================
>
> Name: Tests-cwp.281
> Author: cwp
> Time: 1 January 2014, 1:21:21.324 pm
> UUID: bd0290d9-b157-4355-8e18-8dd4f4705e23
> Ancestors: Tests-fbs.280, Tests-cwp.280
>
> merge
>
> =============== Diff against Tests-fbs.280 ===============
>
> Item was added:
> + ----- Method: AliasTest>>testSource (in category 'tests') -----
> + testSource
> +       | alias source |
> +       source := #Griffle => value.
> +       alias := source asBinding: #Plonk.
> +       self assert: alias source == source!
>
> Item was changed:
>   TestCase subclass: #BindingPolicyTest
> +       instanceVariableNames: 'environment policy value notified notifiedBinding'
> -       instanceVariableNames: 'namespace policy value'
>         classVariableNames: ''
>         poolDictionaries: ''
>         category: 'Tests-Environments'!
>
> Item was removed:
> - ----- Method: BindingPolicyTest class>>isAbstract (in category 'as yet unclassified') -----
> - isAbstract
> -       ^ self name = #BindingPolicyTest!
>
> Item was added:
> + ----- Method: BindingPolicyTest>>addBinding: (in category 'emulating') -----
> + addBinding: aBinding
> +       notified := #add.
> +       notifiedBinding := aBinding!
>
> Item was added:
> + ----- Method: BindingPolicyTest>>bindingPolicyWithNamePolicy: (in category 'tests') -----
> + bindingPolicyWithNamePolicy: aPolicy
> +       ^ BindingPolicy
> +               environment: environment
> +               policy: aPolicy
> +               addSelector: #addBinding:
> +               removeSelector: #removeBinding:!
>
> Item was added:
> + ----- Method: BindingPolicyTest>>removeBinding: (in category 'emulating') -----
> + removeBinding: aBinding
> +       notified := #remove.
> +       notifiedBinding := aBinding!
>
> Item was changed:
>   ----- Method: BindingPolicyTest>>setUp (in category 'running') -----
>   setUp
> +       environment := Environment withName: #test.
> -       namespace := IdentityDictionary new.
>         value := Object new!
>
> Item was added:
> + ----- Method: BindingPolicyTest>>tearDown (in category 'running') -----
> + tearDown
> +       environment destroy.
> +       environment := nil.
> +       value := nil.!
>
> Item was added:
> + ----- Method: BindingPolicyTest>>testAddHonorsEnvironment (in category 'tests') -----
> + testAddHonorsEnvironment
> +       | binding other |
> +       other := Environment withName: #other.
> +       policy := self bindingPolicyWithNamePolicy: AllNamePolicy new.
> +       binding := #Griffle => value.
> +       policy binding: binding addedTo: other notify: self.
> +       self assert: notified = nil!

I think I've established my reputation as a loud lover of
#assert:equals: :) Just sayin'.

What I often do is set these kinds of variables - variables that show
that some side effect took place - to some obviously wrong value. In
this case, you could set notifier := #notCalled in the #setUp.

I can't figure out what this test _does_ though: when you say that add
honours Environment, you mean that it only adds the binding if the
addedTo: environment matches the policy's environment, and we know
this because the notifier isn't called?

> Item was added:
> + ----- Method: BindingPolicyTest>>testRemoveHonorsEnvironment (in category 'tests') -----
> + testRemoveHonorsEnvironment
> +       | binding other |
> +       other := Environment withName: #other.
> +       policy := self bindingPolicyWithNamePolicy: AllNamePolicy new.
> +       binding := #Griffle => value.
> +       policy binding: binding removedFrom: other notify: self.
> +       self assert: notified = nil!

Same thing as #testAddHonorsEnvironment?
> Item was changed:
>   ----- Method: EnvironmentTest>>testImportAddingPrefixResolvesUndeclared (in category 'import tests') -----
>   testImportAddingPrefixResolvesUndeclared
>         | binding foreign |
>         foreign := Environment withName: #Foreign.
>         foreign exportSelf.
> +       foreign bind: #Griffle to: value.
> +       binding := env undeclare: #XXGriffle.

I like this. Nice & explicit language.

> Item was changed:
>   ----- Method: EnvironmentTest>>testImportAliases (in category 'import tests') -----
>   testImportAliases
>         | foreign v2 v3 |
>         foreign := Environment withName: #Foreign.
>         foreign exportSelf.
>         foreign at: #Griffle put: value.
>         foreign at: #Nurp put: (v2 := Object new).
>         foreign at: #Ziffy put: (v3 := Object new).

Shouldn't these be #bind:to: now?

> +       env from: foreign import: {#Nurp -> #Plonk. #Ziffy -> #Wiffy}.
> -       env from: foreign import: {#Plonk -> #Nurp. #Wiffy -> #Ziffy}.

I find this hard to understand. So the Array of Associations looks
like it says "when you see Ziffy, interpret that as if you saw Wiffy",
or "map Ziffy to Wiffy". Oh. It says "when you import foreign, map
foreign's Ziffy to env's (new) Wiffy. OK, that makes sense.

>         self assert: (env bindingOf: #Griffle) isNil.
>         self assert: (env bindingOf: #Plonk) value == v2.
>         self assert: (env bindingOf: #Wiffy) value == v3!

> Item was changed:
>   ----- Method: EnvironmentTest>>testImportWritable (in category 'import tests') -----
>   testImportWritable
>         | foreign binding |
>         foreign := Environment withName: #Foreign.
>         foreign exportSelf.
> +       foreign bind: #Griffle to: 'v1'.
> +       env from: foreign import: #Griffle -> #Plonk.
> -       foreign at: #Griffle put: 'v1'.
> -       env from: foreign import: #Plonk -> #Griffle.
>         binding := env bindingOf: #Plonk.
>         binding value: 'v2'.
> +       self assert: (foreign declarationOf: #Griffle) value = 'v2' !
> -       self assert: (foreign bindingOf: #Griffle) value == 'v2' !

I realise this is existing behaviour, but I'm not so keen on
sub-environments being able to tinker with parent environments'
bindings. What's the use case here? It means an Environment can't be
used as a sandbox.

> Item was added:
> + ----- Method: EnvironmentTest>>testUndeclare (in category 'binding tests') -----
> + testUndeclare
> +       | one two |
> +       one := env undeclare: #Griffle.
> +       two := env bindingOf: #Griffle.
> +       self assert: one == two.
> +       self assert: one class == Global!

Ah. This shows that you can always add new undeclared stuff to an
environment. The name of the test didn't tell me that :/. Also, is it
part of the API that one value isNil?

> Item was added:
> + ----- Method: EnvironmentTest>>testUndeclaredBecomeClassBinding (in category 'compatibility tests') -----
> + testUndeclaredBecomeClassBinding
> +       | binding class |
> +       class := Behavior new.
> +       binding := env undeclared
> +               add: (#Griffle => nil);
> +               associationAt: #Griffle.
> +       env at: #Griffle put: class.
> +       self assert: (binding class == ClassBinding).
> +       self assert: binding value == class.!
>
> Item was added:
> + ----- Method: EnvironmentTest>>testUndeclaredBecomesGlobal (in category 'compatibility tests') -----
> + testUndeclaredBecomesGlobal
> +       | binding class |
> +       class := Behavior new.
> +       binding := env undeclared
> +               add: (#Griffle => class);
> +               associationAt: #Griffle.
> +       env bind: #Griffle to: value.
> +       self assert: (binding class == Global).
> +       self assert: binding value == value.!

Minor nit: one of these tests says "BecomeSomething" and the other
says "BecomesSomething".

> Item was changed:
> + ----- Method: MCClassDefinitionTest>>testLoadAndUnload (in category 'as yet unclassified') -----

So all this noise in the Monticello tests is because there are two
Tests-fbs.280 - one in trunk, and one in the inbox. Fun times. The one
in the inbox changes the MC tests to load definitions into an
Environment that's then thrown away after the test runs. This avoids
MC mucking around with global state.

The changes have been sitting in the Inbox for a week, but given
Colin's big chunk of stuff landing in the Inbox, let's review it, push
to trunk when ready, and then I'll update my MC test hacking, and
resubmit.

frank


More information about the Squeak-dev mailing list