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

RESOLVED FIXED in mozilla17

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

5 years ago
Assignee: nobody → mh+mozilla
(Assignee)

Comment 1

5 years ago
Created attachment 644390 [details] [diff] [review]
Make mozunit even easier to use, and use it in config/tests python unit tests
Attachment #644390 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 2

5 years ago
Created attachment 644853 [details] [diff] [review]
part 2 - Add an enhanced MockedOpen facility to mozunit.py
Attachment #644853 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 3

5 years ago
Created attachment 644854 [details] [diff] [review]
part 2 - Add an enhanced MockedOpen facility to mozunit.py
Attachment #644854 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #644853 - Attachment is obsolete: true
Attachment #644853 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 4

5 years ago
Created attachment 644855 [details] [diff] [review]
part 3 - Use the new MockedOpen in unit-Preprocessor.py
Attachment #644855 - Flags: review?(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!
Attachment #644390 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Updated

5 years ago
Depends on: 776046
(Assignee)

Comment 6

5 years ago
Created attachment 645221 [details] [diff] [review]
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.
Attachment #645221 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #644390 - Attachment is obsolete: true
Attachment #645221 - Flags: review?(ted.mielczarek) → review+
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?
Attachment #644854 - Flags: review?(ted.mielczarek) → review+
Attachment #644855 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 8

5 years ago
> @@ +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
(Assignee)

Comment 9

5 years ago
Created attachment 645317 [details] [diff] [review]
part 2 - Add an enhanced MockedOpen facility to mozunit.py

With support for 'a'.
Attachment #645317 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #644854 - Attachment is obsolete: true
(Assignee)

Comment 10

5 years ago
Created attachment 645319 [details] [diff] [review]
part 3 - Use the new MockedOpen in unit-Preprocessor.py

Refreshed after bug 776046.
(Assignee)

Updated

5 years ago
Attachment #644855 - Attachment is obsolete: true
(Assignee)

Comment 11

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

Carrying over r+.
Attachment #645319 - Flags: review+
Attachment #645317 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 12

5 years ago
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
Target Milestone: --- → mozilla17
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
(Assignee)

Comment 14

5 years ago
Relanded, as it was innocent
https://hg.mozilla.org/integration/mozilla-inbound/rev/284ee7c9fb33
(Assignee)

Comment 15

5 years ago
part 2 & 3:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c94f0e69113c
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c78a8cc97bc
https://hg.mozilla.org/mozilla-central/rev/284ee7c9fb33
https://hg.mozilla.org/mozilla-central/rev/c94f0e69113c
https://hg.mozilla.org/mozilla-central/rev/4c78a8cc97bc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.