891337 - [mozversion] Provide information about the application locale

Status: RESOLVED WONTFIX

(Testing :: Mozbase, defect)

Description

[Andrei Eftimie]

We'll sometimes want to selectively run or skip tests based on the Firefox version used.
ATM we can do that in-test, but there is no way to filter based on version at manifest level.

What we want is to be able to specify in our manifest files something like:
> [sampleTest.js]
> skip-if = app-version <= 25 = "My message for skipping"

This would skip the specified test for Firefox versions older than 25

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

This can not only be used for Firefox but has to work in general for all Gecko based applications. So we need information about the application and platform version.

Comment 2

[Andrei Eftimie]

I'm not sure how we could tackle this.

mozinfo collects information at the OS level, it doesn't know anything about any gecko app
mozrunner _might_ be able to output the info we want here

Maybe we could add support for 'app_version' in mozrunner, and update it once we are running the app from within mozmill (or maybe directly from mozrunner)

Comment 3

[Andrew Halberstadt [:ahal]]

Mozinfo is basically just a glorified dictionary, there is purposefully very very little that the module calculates on it's own (and it's all platform related). Instead consumers of mozinfo can call mozinfo.update(my_dict) to pass in values like Gecko version. Note that the update method updates mozinfo globally to the interpreter.

I think what you want to do is have some kind of populate mozinfo logic that runs before the tests do, and it will be this code that is responsible for getting the Gecko version etc.

Example:
For harnesses that live in tree, we create a file called mozinfo.json that lives in the objdir (see http://mxr.mozilla.org/mozilla-central/source/config/writemozinfo.py). It is then up to the test harnesses to update mozinfo with the data stored in mozinfo.json (see http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py#758).

Comment 4

[Andrei Eftimie]

Thanks Andrew, I think I just realised that I'm supposed to do something very similar as what you're describing.

Thanks for the added info.

Comment 5

[Andrew Halberstadt [:ahal]]

No problem, though I don't think this bug should be in the mozbase component as mozbase shouldn't be changing at all.

Comment 6

[Andrei Eftimie]

I have changed this to the Mozmill component.

ATM we are using custom code to do test-skipping: https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L390

Since we are already doing this, and not relying on mozbase for this, it might make sense to have this feature here.

However, there are some problems / things we need to agree upon:

1) The manifestparser does not have support for the conditions we want (smaller than, greater than). Here are the conditions the parser has support for: https://github.com/mozilla/mozbase/blob/master/manifestdestiny/manifestparser/manifestparser.py#L166

2) In mozmill we do an initial filtering of the tests based on the manifestparser here: https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L777

At this stage the runner is not completely initialised, and we don't have access to the application_version.
We do however have access to this info later on when we start the runner, we can update mozinfo with the relevant info at that time, and then use that info to evaluate the condition and disable a test in the run method:
https://github.com/mozilla/mozmill/blob/master/mozmill/mozmill/__init__.py#L390

=======

I'll shortly upload a patch to highlight where these changes might be made.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

No, this not related to Mozmill only. It really is around Mozrunner. So what we most likely want to have fixed first is bug 769606.

Andrei, I don't see any necessity that we work at this bug atm. There are more important tasks which have to be implemented/fixed first.

Comment 8

[Andrei Eftimie]

Attachment: possible approach

A possible route we can take.

But there are still some issues:

1) We don't know the application_version until we start the runner, and by then its already to late to skip the whole test. (in this patch the condition is evaluated agains a hardcoded value of '25'. this would be taken from mozinfo)

We could have a dummy run, just to collect this info, so we'll know to skip tests after that. And only then start running the actual tests we want.

2) Should we enhance the manifestparser with more conditions?
We would need <, >, <=, >=
Or should we hack these into mozmill?

We are not using the full potential of the manifestparser anyway, since we need to manually evaluate these conditions after we've read the .ini file. But as shown here we can use the ExpressionParser directly.

I would greatly appreciate some feedback on which way to take this.
Thanks.

Comment 9

[Andrei Eftimie]

> Andrei, I don't see any necessity that we work at this bug atm. There are
> more important tasks which have to be implemented/fixed first.

Yeah, will leave this as it is for now as it's more complex than I've initially thought

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 773150 [details] [diff] [review]
possible approach

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

I would prefer to enhance ManifestParser to support greater/less expressions, and use the existing skip-if rather than creating our own disabled-if.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 773150 [details] [diff] [review]
possible approach

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

(In reply to Andrei Eftimie from comment #8)
> 1) We don't know the application_version until we start the runner, and by
> then its already to late to skip the whole test. (in this patch the
> condition is evaluated agains a hardcoded value of '25'. this would be taken
> from mozinfo)

This is not true! All the information we need is located inside the application.ini file.

Comment 12

[Andreea Matei [:AndreeaMatei]]

As discussed today, we would need to have the locale information as well in order to be able to skip a test (or run it) if it's a certain locale.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Basically with bug 928452 we got all the application details except for the locale, which is harder to fetch. Actually I'm not sure how to do this best at the moment. But it is still a feature we would like to have. So I'm going to update the summary for reflection.

I think that Andrei is not going to work on it given that the bug lays around for a while now.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Oh, see the following link for information we can retrieve via mozversion now:

https://github.com/mozilla/mozbase/commit/6baa74e8b71374a2279a3b478617449adcd38423#diff-5cfd0d3d4863c03e025694e003585ad5R48

Comment 15

[Geoff Brown [:gbrown]]

Mass closing bugs with no activity in 2+ years. If this bug is important to you, please re-open.