Closed Bug 1543220 Opened 2 years ago Closed 1 year ago

Make gTests run in automation


(Thunderbird :: Build Config, defect)

Not set


(Not tracked)

Thunderbird 71.0


(Reporter: jorgk-bmo, Assigned: rjl)


(Blocks 1 open bug)



(3 files, 4 obsolete files)

Now that M-C is top source directory, we can run gTests. We have two gTests: TestMsgStripRE and TestMailCookie.

The former will land in bug 1466748 and the latter needs to be repaired in bug 1543219 before it will pass.

To run them:

mach gtest TestMailCookie*
mach gtest TestMsgStripRE*

It would be nice to write a whole lot more gTests, so they should become part of the testing suite.

Flags: needinfo?(rob)

I found another ancient C++ test which could be converted to a gTest:

Assignee: nobody → rob
Flags: needinfo?(rob)
Component: Testing Infrastructure → Build Config
Attached patch gtests_in_ci_wip.patch (obsolete) — Splinter Review
Work in progress.
Attached patch gtests_in_ci_wip.patch (obsolete) — Splinter Review
Work in progress.
Attachment #9091279 - Attachment is obsolete: true

Nice to see. Note that TestMailCookie will currently fail, bug 1579698, after we spent so much energy on making it work in bug 1543219 :-(

Depends on: 1579698

To narrow down the tests that get run for Thunderbird, we can set the "GTEST_FILTER" environment variable, which is what "mach gtest" does. That would require that we stick to a standard prefix or a few prefixes that can be matched with simple wildcards.

Just now I tried "TestMsg*:TestMail*" and it ran the two tests that are set up.

Would that work? There might be some other ways to do it with some C programming.

I'm cool with "TestMsg*:TestMail*". That looks "mail-related".

Pushed by
Enable building gtest xul for Thunderbird. r=nalexander
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0

Hmm, I guess only the M-C part landed here. And I hope we're not busted without the C-C part.

Resolution: FIXED → ---

Thc M-C change just creates the target.gtest.tests.tar.gz artifact during the build so it can be used later for the test.
Unfortunately, setting the environment variables didn't work when I ran my try job late last night. Mozharness saw it just fine and the logs show that GTEST_FILTER is set throughout the setup to run the tests, but then either drops or ignores it, so every test ran. I'll try something else.

Attached patch thunderbird_gtests.patch (obsolete) — Splinter Review
This will get things going. The M-C portion already landed.

I did some sorting as well.
Attachment #9092247 - Flags: review?(geoff)
Attachment #9091281 - Attachment is obsolete: true
Comment on attachment 9092247 [details] [diff] [review]

Review of attachment 9092247 [details] [diff] [review]:

Okay, but I have some (rhetorical) questions.

::: mozharness/unittests/
@@ +17,5 @@
>      ],
> +    "all_gtest_suites": {
> +        "gtest": {
> +            'env': {
> +                'GTEST_FILTER': 'TestMail*:TestMsg*'

Switching quote style?

::: taskcluster/ci/test/compiled.yml
@@ +25,5 @@
> +    treeherder-symbol: GTest
> +    instance-size: xlarge
> +    run-on-projects:
> +        by-test-platform:
> +            windows.*-shippable/.*: []  # possible permafail?

What about non-shippable? Does that not also fail?

::: taskcluster/ci/test/test-platforms.yml
@@ +24,3 @@
>          - mozmill-tests
>          - xpcshell-tests
> +        - marionette-tests

Why does marionette move for each of these? Are you sorting the lists? Because if so they're still very unsorted.
Attachment #9092247 - Flags: review?(geoff) → review+
Attached patch thunderbird_gtests.patch (obsolete) — Splinter Review
No major changes here, just addressing comments.

I had not run the windows-shippable tests yet, that comment relates only
to Firefox it seems, I suspect because they use PGO on shippable builds
and as of now Thunderbird does not.

However, TestMsgStripRE is failing on OSX. It appears to be a legitimate
fail, not something related to automation.

All tests that are not DISABLED will run with this revision. I can make
a followup to disable macOS if the sheriffs like.
Attachment #9092247 - Attachment is obsolete: true
Keywords: checkin-needed

Yes, let's disable whatever is failing. No idea why TestMsgStripRE is failing on Mac. Some more interference from the Mac address book like we had for Marionette? We also need to disable the MailCookie test, see bug 1579698.

Disable macOS.
There's a patch on bug 1579698 to disalble MailCookie tests.
Attachment #9093139 - Attachment is obsolete: true

Geoff, if you land this, please also land bug 1579698. Adding NI so the comment doesn't get overlooked.

Flags: needinfo?(geoff)

Pushed by
Add gtest tests to Taskcluster. r=darktrojan

Closed: 1 year ago1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED

Tests were supposed to be switched off on Mac, but they ran and failed :-(

Flags: needinfo?(geoff) → needinfo?(rob)
Pushed by
Follow-up: fix indentation/yaml error. rs=white-space-only
Blocks: 1581762

(In reply to Jorg K (GMT+2) (reduced availability 14-19 of Sept.) from comment #19)

Tests were supposed to be switched off on Mac, but they ran and failed :-(

I will do a followup this afternoon.

Flags: needinfo?(rob)
Regular Expressions != Globs
Attachment #9093412 - Flags: review?(geoff)
Comment on attachment 9093412 [details] [diff] [review]

I've seen a regexp before in my life.
Attachment #9093412 - Flags: review?(geoff) → review+
Pushed by
Follow-up: Disable gTests on macOS, fix regular expression. r=jorgk
You need to log in before you can comment on or make changes to this bug.