1463702 - TEST-UNEXPECTED-FAIL | services/common/tests/unit/test_remote_settings_jexl_filters.js and TEST-UNEXPECTED-FAIL | services/common/tests/unit/test_blocklist_clients.js

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

EST-UNEXPECTED-FAIL | services/common/tests/unit/test_remote_settings_jexl_filters.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | services/common/tests/unit/test_remote_settings_jexl_filters.js | test_returns_entries_where_jexl_is_true - [test_returns_entries_where_jexl_is_true : 71] 0 == 5
TEST-UNEXPECTED-FAIL | services/common/tests/unit/test_blocklist_clients.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | services/common/tests/unit/test_blocklist_clients.js | test_entries_are_filtered_when_jexl_filters_is_present - [test_entries_are_filtered_when_jexl_filters_is_present : 293] 2 == 4

M-C last good: 96a6ea5ea3468b4c9e20ff8d9795a7ef13
M-C first bad: d36cd8bdbc5c0df1d1d7a167f5fedb95c3

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=96a6ea5ea3468b4c9e20ff8d9795a7ef13&tochange=d36cd8bdbc5c0df1d1d7a167f5fedb95c3

Looks like
Mathieu Leplatre - Bug 1451031 - Add JEXL filter support in Remote Settings r=mythmon
https://hg.mozilla.org/mozilla-central/rev/13f0a3149181

Mathieu, Gijs: Any hints? These tests are disabled for Android, so should they also be disabled for TB?

Comment 1

[:Gijs (he/him)]

Mathieu will know, but I don't. :-)

Comment 2

[Mathieu Leplatre [:leplatrem]]

We had the same error on Android, I disabled them there.

I don't explain why JEXL filters would not resolve to proper values in Thunderbird xpcshell tests. 
Maybe because of some missing JavaScript sandboxing features or something?

Jorg, I'm sorry that once again broke your tests.

You can disable them on TB too, like I did here:
https://reviewboard.mozilla.org/r/243392/diff/3-4/

Or we can investigate why the JEXL engine always return undefined on TB.

For the latter, what do obtain when you execute those lines in a console?

```
ChromeUtils.defineModuleGetter(this, "FilterExpressions", "resource://normandy/lib/FilterExpressions.jsm");

FilterExpressions.eval("a == b", {a: 1, b: 1})
   .then(r => console.log(r));
```

I suspect the problem to be this:
https://searchfox.org/mozilla-central/rev/eb51d734a8c06ac5973291be9bcc74ed4e999bca/toolkit/components/moz.build#113-114

If that's the case, one possible solution is to extract this FilterExpressions module to toolkit/components/utils like we did for Client environment in Bug 1451033

Comment 3

[Jorg K (CEST = GMT+2)]

Thanks for the quick reply. No problem that those tests fail now, if we address it in a timely manner.

ChromeUtils.defineModuleGetter(this, "FilterExpressions", "resource://normandy/lib/FilterExpressions.jsm");
FilterExpressions.eval("a == b", {a: 1, b: 1}).then(r => console.log(r));
returns
[Exception... "Component is not available"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1"  data: no]

I'm happy to summit a patch to disable those tests, unless you want to refactor it as indicated. Please let me know. Also, being the "jack of all trades, master of none" in TB, I don't know that JEXL is about. Something useful for TB? If so, the refactoring might be better, if not, we could just switch off the tests.

Comment 4

[:Gijs (he/him)]

Can we move the jexl stuff into toolkit so that it also ships on non-browser apps? That seems like the most straightforward fix here.

Comment 5

[Mathieu Leplatre [:leplatrem]]

Yes, I can work on that.

Comment 6

[Mathieu Leplatre [:leplatrem]]

I submitted a patch for Bug 1464006, now waiting for checkin :)

Comment 7

[Jorg K (CEST = GMT+2)]

Thanks a lot, I've been watching it. Don't you have Level 3 rights to push it yourself?

Comment 8

[Jorg K (CEST = GMT+2)]

Fixed by bug 1464006. Thanks again, Mathieu!