881417 - add a method to mozinfo to easily find and load mozinfo.json

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[(not currently active) Ted Mielczarek]

Attachment: Add mozinfo.find_and_update_from_json

This patch adds mozinfo.find_and_update_from_json, which takes a list of directories. It invokes a little bit of magic, trying to import the "mozbuild.base" module which only exists in an objdir, and locating mozinfo.json in the root of the objdir using that. If it can't do that it falls back to looking for mozinfo.json in the directories passed to it.

We should be able to invoke this in all our test harnesses and have them "Just work" if we package mozinfo.json next to the harness in the test package.

The name is a bit verbose, I'm open to bikeshedding. Also I added some unit tests for mozinfo.

Comment 1

[Jeff Hammel]

Comment on attachment 760539 [details] [diff] [review]
Add mozinfo.find_and_update_from_json

basically lgtm.  I am wondering if the function should return something as an indicator of whether the file is found and loaded vs not, as currently there is no indication of whether this actually updates or not (let alone what).  You could do True/False for whether an update occurs or not, though I'd recommend the path to which file was actually used or None otherwise.

If this is a good idea, I'm fine taking here (no rereview necessary) or filing a follow-up.

Comment 2

[(not currently active) Ted Mielczarek]

Good point. I had thought about that but just didn't do it. Returning the file name that was used seems sensible. I'll make that change before pushing.

Comment 3

[(not currently active) Ted Mielczarek]

I made that change and fixed the tests. I also tweaked the docstrings a little, and made some changes so the generated docs come out nicer.
https://github.com/mozilla/mozbase/commit/005add096183c50af6a41e172c7727e088fda62a

...but of course one of those last minute changes broke the tests (only when running from the top-level testrunner), so I pushed a followup fix:
https://github.com/mozilla/mozbase/commit/bde4f9106d0d244718f15be84e934b85a9141a42

Comment 4

[Jeff Hammel]

Beh, I should have looked at the test more closely (TBH, for reasons of expedience I no longer recall, I quickly scanned the test).  It uses mock, which isn't in the python stdlib.  I'll file a follow-up bug to figure out what we want to do with test 3rd party deps.

Comment 5

[(not currently active) Ted Mielczarek]

Sorry, I didn't think it was a really big deal since it was test-only. That is kind of a PITA though.

Comment 6

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #4)
> Beh, I should have looked at the test more closely (TBH, for reasons of
> expedience I no longer recall, I quickly scanned the test).  It uses mock,
> which isn't in the python stdlib.  I'll file a follow-up bug to figure out
> what we want to do with test 3rd party deps.

bug 884344

Comment 7

[Jeff Hammel]

:ted, i am a bit confused about the follow-up, https://github.com/mozilla/mozbase/commit/bde4f9106d0d244718f15be84e934b85a9141a42#mozinfo/mozinfo/__init__.py ; I haven't played around with this but....isn't this, um, odd in a few different ways?  IMHO, it'd at least be nice to have a comment as to how/why this is done for future reference.

Also, the reload pattern used in the functions would be nice to have a comment or two as to why.  (TBH, I'd do this as a decorator.) https://bug885145.bugzilla.mozilla.org/attachment.cgi?id=765118 undoes this, so it should probably be figured out.

Comment 8

[(not currently active) Ted Mielczarek]

The reload stuff in the unit test is probably not useful. I was trying to get mozinfo back to a clean state between tests, but that proved futile.

The followup bit is just silliness. Our pattern of mozfoo/__init__.py simply doing "from mozfoo import *" means that sphinx can't auto-doc modules, which is annoying. The "__all__ = mozinfo.__all__" bit made sphinx happy, but I neglected to import the mozinfo name in __init__ which broke things. (Somehow it didn't break in my local testing, must have been a quirk of module loading.)

Comment 9

[(not currently active) Ted Mielczarek]

Probably what I want there is a relative import, "from .mozinfo import __all__".