Closed Bug 1341015 Opened 3 years ago Closed 3 years ago

Document how to build the DLLs for the "modules" ping tests

Categories

(Toolkit :: Telemetry, defect, P3)

defect
Points:
2

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Dexter, Assigned: marco)

References

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

In bug 1330833 some test DLLs were landed to allow testing for the following cases (see bug 1330833 comment 44 for more context):

1) PDB missing;
2) PDB containing unicode characters;
3) DLL name containing unicode characters.

Instead of checking in the DLL binaries, along with the source code, we should generate them as part of the build process from the source. An example of this can be found at [1].

[1] - https://dxr.mozilla.org/mozilla-central/source/build/win32/crashinjectdll/moz.build
Blocks: 1330833
Points: --- → 2
Priority: -- → P2
Whiteboard: [measurement:client]
I've generated one of them as part of the build process: https://hg.mozilla.org/integration/mozilla-inbound/diff/8fc28f2d52d9/toolkit/components/telemetry/tests/moz.build.

Unfortunately it isn't possible for all of them, as the build system doesn't support Unicode well (and there are no plans to do it).

It might be possible to generate the "PDB missing" one through the build system.
Ted, do you know if we are ok with (some) binaries in-tree in this case?
Flags: needinfo?(ted)
Note that we do already have other `.dll` and `.exe` files in-tree (more often `.exe` than `.dll`):
https://dxr.mozilla.org/mozilla-central/search?q=file%3A.exe&redirect=false
https://dxr.mozilla.org/mozilla-central/search?q=file%3A.dll&redirect=false
(In reply to Georg Fritzsche [:gfritzsche] from comment #2)
> Ted, do you know if we are ok with (some) binaries in-tree in this case?

I think as long as they're effectively just test data, and we have the source and a means to rebuild them if necessary checked-in alongside them it's fine.
Flags: needinfo?(ted)
Do we have documentation on how to rebuild the binaries for the dlls we can't generate through the build system? If not, would you kindly add it?
Flags: needinfo?(mcastelluccio)
Priority: P2 → P3
The source (if we can call it that :P) is the same as the libmodules-test lib (https://hg.mozilla.org/integration/mozilla-inbound/file/8fc28f2d52d9/toolkit/components/telemetry/tests/modules-test.cpp).

Where should we add the documentation? In modules-test.cpp, in https://hg.mozilla.org/integration/mozilla-inbound/diff/8fc28f2d52d9/toolkit/components/telemetry/tests/moz.build, at https://hg.mozilla.org/integration/mozilla-inbound/file/8fc28f2d52d9/toolkit/components/telemetry/tests/unit/xpcshell.ini#l17, or in test_TelemetryModules.jsm?
I propose in xpcshell.ini and in test_TelemetryModules.jsm.
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Flags: needinfo?(mcastelluccio)
(In reply to Marco Castelluccio [:marco] from comment #6)
> The source (if we can call it that :P) is the same as the libmodules-test
> lib
> (https://hg.mozilla.org/integration/mozilla-inbound/file/8fc28f2d52d9/
> toolkit/components/telemetry/tests/modules-test.cpp).
> 
> Where should we add the documentation? In modules-test.cpp, in
> https://hg.mozilla.org/integration/mozilla-inbound/diff/8fc28f2d52d9/toolkit/
> components/telemetry/tests/moz.build, at
> https://hg.mozilla.org/integration/mozilla-inbound/file/8fc28f2d52d9/toolkit/
> components/telemetry/tests/unit/xpcshell.ini#l17, or in
> test_TelemetryModules.jsm?
> I propose in xpcshell.ini and in test_TelemetryModules.jsm.

Let's add a comment in test_TelemetryModules.jsm please!
For things like this, a comment in the test file pointing to the source file the library was built from, and then a comment (or comments) in the source file with the compiler commands to build the library should be fine. Rebuilding it ought to be an infrequent occurrence (possibly nobody will ever need to), but at least we'll know how to do so. :)
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)
> For things like this, a comment in the test file pointing to the source file
> the library was built from, and then a comment (or comments) in the source
> file with the compiler commands to build the library should be fine.
> Rebuilding it ought to be an infrequent occurrence (possibly nobody will
> ever need to), but at least we'll know how to do so. :)

Awesome, thanks Ted!
Attached patch PatchSplinter Review
Attachment #8847688 - Flags: review?(alessio.placitelli)
Attachment #8847688 - Flags: review?(alessio.placitelli) → review+
Summary: Generate the test dlls for the "modules" ping from the source → Document how to build the DLLs for the "modules" ping tests
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/095c4f971c19
Document how to build the shared libraries needed for the TelemetryModules test. r=Dexter
https://hg.mozilla.org/mozilla-central/rev/095c4f971c19
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.