./mach test gtest && ./mach gtest test-pathes is not work || ./mach test test-pathes will not invoke gtest

NEW
Unassigned

Status

4 years ago
5 days ago

People

(Reporter: u459114, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 8 obsolete attachments)

(Reporter)

Description

4 years ago
1. "./mach test" command is not work for gtest
> cjcool@~/repository/gecko-dev (git/master u= origin/master)$ ./mach test gfx/test/gtest
> UNKNOWN TEST: gfx/tests/gtest
> I was unable to find tests in the argument(s) given.
(Looks like gtest-cases are not put in all-tests.json)

2. ./mach gtest path-to-gtest-folder
> cjcool@~/repository/gecko-dev (git/master u= origin/master)$ ./mach gtest gfx/tests/gtest
> ...
> Running GTest tests...
> Note: Google Test filter = gfx/tests/gtest
> [==========] Running 0 tests from 0 test cases.
> [==========] 0 tests from 0 test cases ran. (1 ms total)
> [  PASSED  ] 0 tests.
> Finished running GTest tests.
There is no test cases been executed.

It makes gtests unique among testing suites. I have to explicit choose test case name pattern as "mach test" command argument.

As a user, when I submit the following command -
  $ ./mach test gfx
I expect running through all testing suites, includes reftest/ mochitest and gtest, under gfx folder.

We might need to generate a map of gtest in build process for this purpose. Any suggestion?

PS: all-tests.json is really huge... The size of this file is around 45MB, while only one-line in this file. I can't open this file and do a search in any editors.
This is tricky for a number of reasons right now. For one, the only thing moz.build knows about gtest sources is that there are source files with FINAL_LIBRARY = 'xul-gtest':
http://dxr.mozilla.org/mozilla-central/source/dom/media/gtest/moz.build#41

We could swap this out for something that's more visible to moz.build, that wouldn't be too bad.

The second part that's tricky is that gtest filtering uses test names (as defined in source files), which have nothing to do with the path to the source files in the tree:
http://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mach_commands.py#660

For example, to run only this media test:
http://dxr.mozilla.org/mozilla-central/source/dom/media/gtest/TestAudioCompactor.cpp#84

You'd specify "Media::AudioCompactor_4000". I don't know how we could possibly make mach aware of this short of instrumenting the build in some way.
(Reporter)

Updated

4 years ago
Assignee: nobody → cku
(Reporter)

Comment 2

4 years ago
FINAL_LIBRARY = 'xul-gtest' gives us a hint there are test cases in this context(moz.build), but we still don't know the name of test cases in Context.SOURCES.

One idea is adding one more property, e.g. Context.GTESTCASENAME, in Context.
In moz.build, developers need to manually write down test case name in this folder.
Here is an example, in dom/media/gtest/moz.build, insert the following line
  GTESTCASENAME = (Media, GeckoMediaPlugins, MP4Demuxer, MP4Reader, VideoSegment, WebMBuffered) 


Create a map between CONTEXT.SOURCES and CONTEXT.GTESTCASENAME and store this map in a storage file.
You can be sure this will be badly maintained and will often not match reality. I'd rather have something automatic, like reading the source files and looking for TEST(testname, subtestname) patterns. Gtest has plenty of macros that you essentially have to use if you want your tests to work, and all tests should be declared using them, so they should be easy to find.
I was wondering if we could coerce gtest to list all the tests it knows about and their corresponding source files, but it doesn't appear to even store the source file information, so that's unlikely to work.
We could look at the symbol table for xul-gtest and run a regex on it. If it doesn't we can make sure that the function name emmited by TEST(a,b) patch a simple regex, but they might already. Something like nm xul-gtest | grep ^gtest_(.*)_(.*)

Better yet we could do it on the object files themselves and not require linking gtest.
(Reporter)

Comment 6

4 years ago
Thanks for all your suggestions

Here is a list of options of make path-filter mapping work
1. (comment 2) implicit add test case name in moz.build.
2. (comment 3) gtest source code parsing(by clang?) 
3. (comment 5) gtest object file parsing(nm/ objdump)
4. Rewrite TEST marco, adding source path attrribute(via __FILE__) into TestInfo.
(Reporter)

Comment 7

4 years ago
(In reply to C.J. Ku[:cjku] from comment #6)
> Here is a list of options of make path-filter mapping work
> 1. (comment 2) implicit add test case name in moz.build.
typo: explicit
(In reply to C.J. Ku[:cjku] from comment #6)
> 2. (comment 3) gtest source code parsing(by clang?) 

Grep or similar "parsing" from python is likely enough
(Reporter)

Comment 9

4 years ago
Created attachment 8542462 [details] [diff] [review]
(WIP) persist sourch-path-to-gtest-filter map

WIP1: search gtest source files and store source-path-to-filter mapping into gtestmap file
(Reporter)

Comment 10

4 years ago
Created attachment 8542478 [details] [diff] [review]
(WIP) persist sourch-path-to-gtest-filter map
Attachment #8542462 - Attachment is obsolete: true
(Reporter)

Comment 11

4 years ago
1. Integrate gtestmap file with make system
2. @Command('gtest'/'test'...): translate path into gtest-filter, and run gtest cases under the specific folder.
(Reporter)

Comment 12

4 years ago
Created attachment 8543527 [details] [diff] [review]
(WIP2) persist sourch-path-to-gtest-filter map
Attachment #8542478 - Attachment is obsolete: true
(Reporter)

Comment 13

4 years ago
Created attachment 8543536 [details] [diff] [review]
(WIP3) persist sourch-path-to-gtest-filter map
Attachment #8543527 - Attachment is obsolete: true
(Reporter)

Comment 14

4 years ago
TBD
1. test case
2. @Command('test'
(Reporter)

Comment 15

4 years ago
If anyone wants to give it a try, here are steps:

1. Apply attachment 8543536 [details] [diff] [review]
2. ./mach build - generate gtestmap file under obj-folger.
3. ./mach gtest gfx - run all test cases in gfx folder
   Or "./mach gtest dom/media" which run through all media recorder gtest cases.
(Reporter)

Comment 16

4 years ago
Created attachment 8543758 [details] [diff] [review]
(WIP3) persist sourch-path-to-gtest-filter map
Attachment #8543536 - Attachment is obsolete: true
(Reporter)

Comment 17

4 years ago
Comment on attachment 8543758 [details] [diff] [review]
(WIP3) persist sourch-path-to-gtest-filter map

Hi Mike, would you mind have a look on this patch?
Attachment #8543758 - Flags: feedback?(mh+mozilla)
(Reporter)

Comment 18

4 years ago
Created attachment 8543806 [details] [diff] [review]
(WIP3) persist sourch-path-to-gtest-filter map

autopep8 gtest_map.py
Attachment #8543758 - Attachment is obsolete: true
Attachment #8543758 - Flags: feedback?(mh+mozilla)
Attachment #8543806 - Flags: feedback?(mh+mozilla)
(Reporter)

Comment 19

4 years ago
Created attachment 8544446 [details] [diff] [review]
(WIP4) Covert a path to gtest filters

Hi, here is a version of grepping gtest source code and create a map file to store relation between source code path and testcase name.
May I have your opinion here? thanks
Attachment #8543806 - Attachment is obsolete: true
Attachment #8543806 - Flags: feedback?(mh+mozilla)
Attachment #8544446 - Flags: feedback?(ted)
Attachment #8544446 - Flags: feedback?(mh+mozilla)
Attachment #8544446 - Flags: feedback?(bgirard)
Comment on attachment 8544446 [details] [diff] [review]
(WIP4) Covert a path to gtest filters

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

::: Makefile.in
@@ +91,4 @@
>  	@$(TOUCH) $@
>  
>  include backend.RecursiveMakeBackend.pp
> +include gtestdeps.mk

gtestdeps is missing from the patch.
(Reporter)

Comment 21

4 years ago
(In reply to Benoit Girard (:BenWa) from comment #20)
> Comment on attachment 8544446 [details] [diff] [review]
> (WIP4) Covert a path to gtest filters
> 
> Review of attachment 8544446 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: Makefile.in
> @@ +91,4 @@
> >  	@$(TOUCH) $@
> >  
> >  include backend.RecursiveMakeBackend.pp
> > +include gtestdeps.mk
> 
> gtestdeps is missing from the patch.

Just like backend.RecursiveMakeBackend.pp, it's a auto-generated file. gtest_mapper.save produce this file after parsing all Contexts
Comment on attachment 8544446 [details] [diff] [review]
(WIP4) Covert a path to gtest filters

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

Are we scanning each file in our tree here? Adding an extra pass on all our sources is not ideal.

::: python/mozbuild/mozbuild/frontend/gtest_mapper.py
@@ +8,5 @@
> +from .context import Context
> +from ..makeutil import Makefile
> +
> +MAP_FILENAME = 'gtestmap'
> +DEP_FILENAME = 'gtestdeps.mk'

If this is a generated dep file you should use a more standard extension IMO. Something like .pp. It's a bit confusing because with .mk I expect to find a Makefile in the tree but with .pp I know what we're generating.

You're only generating dependencies in that file right?

::: python/mozbuild/mozbuild/mach_commands.py
@@ +698,5 @@
>              os.makedirs(cwd)
>  
> +        # Convert gtest_filter from path to filter.
> +        import mozbuild.frontend.gtest_mapper as gm
> +        gtest_filter = gm.load(self.topsrcdir, self.topobjdir, gtest_filter)

NIT: It's usually better to not use the same variable for something that has too different format. I'd recommend having a different name for gtest_path.
Attachment #8544446 - Flags: feedback?(bgirard)
(Reporter)

Comment 23

4 years ago
(In reply to Benoit Girard (:BenWa) from comment #22)
> Comment on attachment 8544446 [details] [diff] [review]
> (WIP4) Covert a path to gtest filters
> 
> Review of attachment 8544446 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Are we scanning each file in our tree here? Adding an extra pass on all our
> sources is not ideal.
Nope, only scan source files whose target lib is xul-gtest.

if not isinstance(context, Context) or \
   'xul-gtest' != context.get('FINAL_LIBRARY'):
   continue << skip file parsing if target lib is not xul-gtest
(Reporter)

Comment 24

4 years ago
Comment on attachment 8544446 [details] [diff] [review]
(WIP4) Covert a path to gtest filters

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

::: python/mozbuild/mozbuild/frontend/gtest_mapper.py
@@ +9,5 @@
> +from ..makeutil import Makefile
> +
> +MAP_FILENAME = 'gtestmap'
> +DEP_FILENAME = 'gtestdeps.mk'
> +

Yes, only dependencies.

Will rename this file according

::: python/mozbuild/mozbuild/mach_commands.py
@@ +699,5 @@
>  
> +        # Convert gtest_filter from path to filter.
> +        import mozbuild.frontend.gtest_mapper as gm
> +        gtest_filter = gm.load(self.topsrcdir, self.topobjdir, gtest_filter)
> +

Understand. Will do
(In reply to C.J. Ku[:cjku] from comment #23)
> (In reply to Benoit Girard (:BenWa) from comment #22)
> > Comment on attachment 8544446 [details] [diff] [review]
> > (WIP4) Covert a path to gtest filters
> > 
> > Review of attachment 8544446 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Are we scanning each file in our tree here? Adding an extra pass on all our
> > sources is not ideal.
> Nope, only scan source files whose target lib is xul-gtest.
> 
> if not isinstance(context, Context) or \
>    'xul-gtest' != context.get('FINAL_LIBRARY'):
>    continue << skip file parsing if target lib is not xul-gtest

Ohh that's clever! I like it.
(Reporter)

Comment 26

4 years ago
Created attachment 8546484 [details] [diff] [review]
(WIP5) Covert path to gtest filters
(Reporter)

Updated

4 years ago
Attachment #8546484 - Flags: feedback?(ted)
Attachment #8546484 - Flags: feedback?(mh+mozilla)
(Reporter)

Updated

4 years ago
Attachment #8544446 - Attachment is obsolete: true
Attachment #8544446 - Flags: feedback?(ted)
Attachment #8544446 - Flags: feedback?(mh+mozilla)
(Reporter)

Comment 27

4 years ago
Created attachment 8547410 [details] [diff] [review]
Part 1. Covert path-paramenter into gtest_filter

Hi Mike/ Gps,
Please help tp review this patch when you have time.

By this change, user can
1.
$ ./mach gtest gfx/test # a test path is given
To run throught all gtest cases in gfx/test folder
2.
./mach gtest -f APZC* # a test filter is given
To run all test cases matched APZC*
3. 
./mach test gfx/test
To run all tests, include gtest, in gfx/test
4.
./mach test gtest
which is equal to './mach gtest -f *' or './mach gtest .'
Attachment #8546484 - Attachment is obsolete: true
Attachment #8546484 - Flags: feedback?(ted)
Attachment #8546484 - Flags: feedback?(mh+mozilla)
Attachment #8547410 - Flags: review?(mh+mozilla)
Attachment #8547410 - Flags: review?(gps)
(Reporter)

Updated

4 years ago
Summary: ./mach test path-to-gtest-folder is not work → ./mach test gtest || ./mach gtest test-pathes is not work || ./mach test test-pathes will not invoke gtest
(Reporter)

Updated

4 years ago
Summary: ./mach test gtest || ./mach gtest test-pathes is not work || ./mach test test-pathes will not invoke gtest → ./mach test gtest && ./mach gtest test-pathes is not work || ./mach test test-pathes will not invoke gtest
Comment on attachment 8547410 [details] [diff] [review]
Part 1. Covert path-paramenter into gtest_filter

I'm busy this week. glandium can look at this.
Attachment #8547410 - Flags: review?(gps)
(Reporter)

Updated

4 years ago
Assignee: cku → nobody
(Reporter)

Updated

4 years ago
Attachment #8547410 - Flags: review?(mh+mozilla)

Updated

10 months ago
Product: Core → Firefox Build System
Component: Mach Core → GTest
Product: Firefox Build System → Testing
You need to log in before you can comment on or make changes to this bug.