776035 - Make mozunit even easier to use, and use it in config/tests python unit tests

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Make mozunit even easier to use, and use it in config/tests python unit tests

Comment 2

[Mike Hommey [:glandium]]

Attachment: part 2 - Add an enhanced MockedOpen facility to mozunit.py

Comment 3

[Mike Hommey [:glandium]]

Attachment: part 2 - Add an enhanced MockedOpen facility to mozunit.py

Comment 4

[Mike Hommey [:glandium]]

Attachment: part 3 - Use the new MockedOpen in unit-Preprocessor.py

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 644390 [details] [diff] [review]
Make mozunit even easier to use, and use it in config/tests python unit tests

Review of attachment 644390 [details] [diff] [review]:
-----------------------------------------------------------------

I forgot this file existed. Very cool!

Comment 6

[Mike Hommey [:glandium]]

Attachment: Make mozunit even easier to use, and use it in config/tests python unit tests

Same as the previous patch, but adds idl-parser and xpt unit tests. Also now depends on virtualenv pth (bug 776046) and removes the sys.path tricks.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 644854 [details] [diff] [review]
part 2 - Add an enhanced MockedOpen facility to mozunit.py

Review of attachment 644854 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/mozunit.py
@@ +108,5 @@
> +    self.assertRaises(Exception,f.open('foo', 'r'))
> +    '''
> +    def __init__(self, files = {}):
> +        self.files = {}
> +        for name, content in files.items():

It's more conventional to use .iteritems().

@@ +113,5 @@
> +            self.files[os.path.abspath(name)] = content
> +
> +    def __call__(self, name, mode = 'r'):
> +        absname = os.path.abspath(name)
> +        if 'w' in mode:

Do you want to handle 'a' here?

@@ +117,5 @@
> +        if 'w' in mode:
> +            return MockedFile(self, absname)
> +        if absname in self.files:
> +            return MockedFile(self, absname, self.files[absname])
> +        return self.open(name, mode)

It seems a little odd that you only mock reads from files explicitly named in the constructor, but you mock writes to all files. I wonder if it'd be better to require explicitly listing all files up front, and assert when a file is opened that wasn't specified?

::: config/tests/unit-mozunit.py
@@ +3,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +import sys
> +import os
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..'))

This shouldn't be necessary with the virtualenv changes, right?

Comment 8

[Mike Hommey [:glandium]]

> @@ +113,5 @@
> > +            self.files[os.path.abspath(name)] = content
> > +
> > +    def __call__(self, name, mode = 'r'):
> > +        absname = os.path.abspath(name)
> > +        if 'w' in mode:
> 
> Do you want to handle 'a' here?

Indeed.

> 
> @@ +117,5 @@
> > +        if 'w' in mode:
> > +            return MockedFile(self, absname)
> > +        if absname in self.files:
> > +            return MockedFile(self, absname, self.files[absname])
> > +        return self.open(name, mode)
> 
> It seems a little odd that you only mock reads from files explicitly named
> in the constructor, but you mock writes to all files. I wonder if it'd be
> better to require explicitly listing all files up front, and assert when a
> file is opened that wasn't specified?

My idea was that using MockedOpen, you can expect that no write will happen on the file system.

> ::: config/tests/unit-mozunit.py
> @@ +3,5 @@
> > +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> > +
> > +import sys
> > +import os
> > +sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
> 
> This shouldn't be necessary with the virtualenv changes, right?

Sure. I didn't refresh this part after I did bug 776046

Comment 9

[Mike Hommey [:glandium]]

Attachment: part 2 - Add an enhanced MockedOpen facility to mozunit.py

With support for 'a'.

Comment 10

[Mike Hommey [:glandium]]

Attachment: part 3 - Use the new MockedOpen in unit-Preprocessor.py

Refreshed after bug 776046.

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 645319 [details] [diff] [review]
part 3 - Use the new MockedOpen in unit-Preprocessor.py

Carrying over r+.

Comment 12

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d56621abfee
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6a0ac7d47d2
https://hg.mozilla.org/integration/mozilla-inbound/rev/62facd5b3da9

Comment 13

[Ed Morley [:emorley]]

Push backed out for Windows mochitest crashes:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=eecd3aa199e6

https://hg.mozilla.org/integration/mozilla-inbound/rev/1abfd50c8be6

Comment 14

[Mike Hommey [:glandium]]

Relanded, as it was innocent
https://hg.mozilla.org/integration/mozilla-inbound/rev/284ee7c9fb33

Comment 15

[Mike Hommey [:glandium]]

part 2 & 3:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c94f0e69113c
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c78a8cc97bc

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/284ee7c9fb33
https://hg.mozilla.org/mozilla-central/rev/c94f0e69113c
https://hg.mozilla.org/mozilla-central/rev/4c78a8cc97bc