Rust code expects test code to be there, breaks --disable-tests

RESOLVED FIXED in Firefox 57

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tjr, Assigned: tjr)

Tracking

(Blocks 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [tor])

Attachments

(1 attachment)

Assignee

Description

2 years ago
Compiling with --disable-tests yields the following:

> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest13nonfatal_failE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:16: undefined reference to `GTest_ExpectFailure'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest18Rust_AssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:60: undefined reference to `Cpp_AssignFromCpp'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest23Rust_FixedAssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:75: undefined reference to `Cpp_AssignFromCpp'
> 51:37.42 ../i686-pc-windows-gnu/release/gkrust_gtest.lib(gkrust_gtest-6cee6ae35964b000.0.o): In function `ZN14nsstring_gtest22Rust_AutoAssignFromCppE':
> 51:37.42 /home/worker/workspace/build/src/xpcom/rust/nsstring/gtest/test.rs:88: undefined reference to `Cpp_AssignFromCpp'

This is because those functions are defined in a test file.
Assignee

Updated

2 years ago
Attachment #8900827 - Flags: review?(ted)
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8900827 [details]
Bug 1393454 Do not compile rust gtest crate if --disable-tests is set

https://reviewboard.mozilla.org/r/172270/#review178720

::: commit-message-e408d:1
(Diff revision 2)
> +Bug 1393454 Do not compile rust tests if --disable-tests is set

nit: this should probably say "Do not compile rust gtest crate", since those are the specific Rust tests in question.

::: moz.configure:275
(Diff revision 2)
>  # on Desktop platforms with the exception of Windows PGO, where linking
>  # xul-gtest.dll takes too long.
>  @depends('MOZ_PGO', build_project, target, 'MOZ_AUTOMATION', '--disable-gtest-in-build',
> -         when='--enable-compile-environment')
> -def build_gtest(pgo, build_project, target, automation, enabled):
> +         enable_tests, when='--enable-compile-environment')
> +def build_gtest(pgo, build_project, target, automation, enabled, enable_tests):
> +    if not enable_tests:

You could combine these two conditionals into a single one. (It's not a big deal, though.)

::: toolkit/toolkit.mozbuild:11
(Diff revision 2)
>  DIRS += [
> -    '/toolkit/library/gtest/rust',
>      '/toolkit/library/rust',
>  ]
>  
> +if CONFIG['ENABLE_TESTS']:

Moving this down below does change the ordering, but I *think* that ought to be fine in practice.
Attachment #8900827 - Flags: review?(ted) → review+
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/08122b5e49f8
Do not compile rust gtest crate if --disable-tests is set r=ted
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/08122b5e49f8
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.