[Test Fixit] Code coverage test example.

[Test Fixit] Code coverage test example.

[Sean Parent, 2010-03-30 20:33:41]

Finally working! The next step is to pull over one of the old unit tests. This
is still a little rough around the edges so any improvements appreciated!

[Sean Parent, 2010-03-30 20:36:00]

Finally working! The next step is to pull over one of the old unit tests. This
is still a little rough around the edges so any improvements appreciated!

[kmixter1, 2010-03-31 01:38:56]

This seems like a good approach.  I have a couple of questions.

Are you planning to submit as an example or are you adding specific unit tests
to this CL and then submitting?

Would you normally be building unit tests this way or is it more likely to be
pulling unit test binaries from somewhere else?

If you do make it an example, could you name it more like existing naming
convention, something like example_UnitTest?

Darin might more comments on the deps setup code.

http://codereview.chromium.org/1597001/diff/1/5
File client/deps/gtest/gtest.py (right):

http://codereview.chromium.org/1597001/diff/1/5#newcode14
client/deps/gtest/gtest.py:14: shutil.copyfile(usr_lib + '/libgtest.a', top_dir
+ '/libgtest.a')
nit: Probably would be easier to maintain to have a list of filenames and loop
through copying them

http://codereview.chromium.org/1597001/diff/1/5#newcode16
client/deps/gtest/gtest.py:16: shutil.copyfile(usr_lib + '/libgtest.so.0.0.0',
top_dir + '/libgtest.so.0.0.0')
80 chars here and elsewhere

http://codereview.chromium.org/1597001/diff/1/8
File client/site_tests/example_unit_test/src/Makefile (right):

http://codereview.chromium.org/1597001/diff/1/8#newcode15
client/site_tests/example_unit_test/src/Makefile:15: $(CXX) $(CXXFLAGS)
$(OPTIONS) $(LDFLAGS) $(PACKAGES) $(OBJECTS) $(LIBS) -o $(TARGET)
80 chars and below

http://codereview.chromium.org/1597001/diff/1/11
File server/site_tests/example_unit_test_server/example_unit_test_server.py
(right):

http://codereview.chromium.org/1597001/diff/1/11#newcode24
server/site_tests/example_unit_test_server/example_unit_test_server.py:24: class
example_unit_test_server(test.test):
Seems like instead of calling this an example, you could pass a parameter to
this class which is the client unit test to run, and this class would just be
the unit test framework class.  Could call it something like framework_Coverage.
 You would still need to know the list of cc files but possibly this is
something the client test could always write out in its result directory.

http://codereview.chromium.org/1597001/diff/1/11#newcode61
server/site_tests/example_unit_test_server/example_unit_test_server.py:61:
self.write_perf_keyval({'gcov': filtered})
filtered is a dictionary right?  I didn't think this would work.

[petkov, 2010-03-31 04:34:42]

http://codereview.chromium.org/1597001/diff/1/5
File client/deps/gtest/gtest.py (right):

http://codereview.chromium.org/1597001/diff/1/5#newcode14
client/deps/gtest/gtest.py:14: shutil.copyfile(usr_lib + '/libgtest.a', top_dir
+ '/libgtest.a')
On 2010/03/31 01:38:56, kmixter1 wrote:
> nit: Probably would be easier to maintain to have a list of filenames and loop
> through copying them

Second that. It seems that in this case you could simply copy libgtest*

I'm a bit confused though... How does gtest get built? It's  either a dependence
of autotest.ebuild (which it seems it's not), or it's a dependence for chromeos
or chromeos-dev, in which case it doesn't need a client/deps because it ends up
on the image anyway...

http://codereview.chromium.org/1597001/diff/1/7
File client/site_tests/example_unit_test/example_unit_test.py (right):

http://codereview.chromium.org/1597001/diff/1/7#newcode43
client/site_tests/example_unit_test/example_unit_test.py:43:
self.write_perf_keyval({'gtest': result.stdout})
what's result.stdout? is it a number? if not, this doesn't seem to work.

http://codereview.chromium.org/1597001/diff/1/8
File client/site_tests/example_unit_test/src/Makefile (right):

http://codereview.chromium.org/1597001/diff/1/8#newcode25
client/site_tests/example_unit_test/src/Makefile:25: #$(TARGET): $(OBJECTS)
It seems you don't need the commented out code -- remove!

http://codereview.chromium.org/1597001/diff/1/11
File server/site_tests/example_unit_test_server/example_unit_test_server.py
(right):

http://codereview.chromium.org/1597001/diff/1/11#newcode12
server/site_tests/example_unit_test_server/example_unit_test_server.py:12: def
GenGCovReport(report, files):
per style, i'd change to all lower-case with underscores.

http://codereview.chromium.org/1597001/diff/1/11#newcode46
server/site_tests/example_unit_test_server/example_unit_test_server.py:46:
report = subprocess.Popen(['gcov', ''.join(test_files)],
use utils.system instead, it's the autotest way.

[Sean Parent, 2010-03-31 17:43:10]

Answering the general questions first - I'll get to specific comments a bit
later.

I'm submitting this as an example. WIth a separate cl I'll submit a specific
instance (and likely clean / factor the example a bit more with it).

Building the unit test this was was Chris Sosa's recommendation. I'm open to
suggestions (not a fan of Make). I didn't want building the unit tests to slow
down the normal build though.

I will rename the directories to follow the convention.

[sosa, 2010-03-31 19:01:34]

http://codereview.chromium.org/1597001/diff/1/5
File client/deps/gtest/gtest.py (right):

http://codereview.chromium.org/1597001/diff/1/5#newcode14
client/deps/gtest/gtest.py:14: shutil.copyfile(usr_lib + '/libgtest.a', top_dir
+ '/libgtest.a')
Didn't you add the build of gtest to the autotest ebuild rdepend or was it
already in the sysroot?

On 2010/03/31 04:34:42, petkov wrote:
> On 2010/03/31 01:38:56, kmixter1 wrote:
> > nit: Probably would be easier to maintain to have a list of filenames and
loop
> > through copying them
> 
> Second that. It seems that in this case you could simply copy libgtest*
> 
> I'm a bit confused though... How does gtest get built? It's  either a
dependence
> of autotest.ebuild (which it seems it's not), or it's a dependence for
chromeos
> or chromeos-dev, in which case it doesn't need a client/deps because it ends
up
> on the image anyway...
>

http://codereview.chromium.org/1597001/diff/1/7
File client/site_tests/example_unit_test/example_unit_test.py (right):

http://codereview.chromium.org/1597001/diff/1/7#newcode4
client/site_tests/example_unit_test/example_unit_test.py:4: 
You may want to make this into a base class that other unittests inherit from. 
This way you could use setup() and initialize() to do the deps stuff and cleanup
to do the copying steps.  This way other unittests could just subclass and not
have to worry about deps at all.

See client/bin/site_ui_test as an example

[kmixter1, 2010-04-01 15:52:31]

Submitting as an example seems like a good start and I don't want to
block that progress.  I think there are some interesting blanks to
fill in between this and an actual example.  For instance, this gives
coverage information on the unit test code itself.  But when you want
to get coverage on the actual production code is the plan to rebuild
that code via autotest outside of the portage build system?

On Wed, Mar 31, 2010 at 10:43 AM,  <seanparent@chromium.org> wrote:
> Answering the general questions first - I'll get to specific comments a bit
> later.
>
> I'm submitting this as an example. WIth a separate cl I'll submit a specific
> instance (and likely clean / factor the example a bit more with it).
>
> Building the unit test this was was Chris Sosa's recommendation. I'm open to
> suggestions (not a fan of Make). I didn't want building the unit tests to
> slow
> down the normal build though.
>
> I will rename the directories to follow the convention.
>
> http://codereview.chromium.org/1597001
>

[Sean Parent, 2010-04-08 18:21:56]

Lots of refactoring and renaming, I read up on python class inheritance and how
to get make to use gcc dependency output.
>>>
For instance, this gives
coverage information on the unit test code itself.  But when you want
to get coverage on the actual production code is the plan to rebuild
that code via autotest outside of the portage build system?
<<<
Yes - that is somewhat necessary. It needs to be build with different options
for coverage and I don't want to slow the current builds. You can't emerge from
inside an autotest so the current preferred way is to use make. As noted above,
I did improve the make example so it will handle include file dependencies
correctly.

With this refactoring a "real" example is coming shortly.

http://codereview.chromium.org/1597001/diff/1/5
File client/deps/gtest/gtest.py (right):

http://codereview.chromium.org/1597001/diff/1/5#newcode14
client/deps/gtest/gtest.py:14: shutil.copyfile(usr_lib + '/libgtest.a', top_dir
+ '/libgtest.a')
On 2010/03/31 04:34:42, petkov wrote:
> On 2010/03/31 01:38:56, kmixter1 wrote:
> > nit: Probably would be easier to maintain to have a list of filenames and
loop
> > through copying them
> 
> Second that. It seems that in this case you could simply copy libgtest*
> 
> I'm a bit confused though... How does gtest get built? It's  either a
dependence
> of autotest.ebuild (which it seems it's not), or it's a dependence for
chromeos
> or chromeos-dev, in which case it doesn't need a client/deps because it ends
up
> on the image anyway...
> 

Done.

http://codereview.chromium.org/1597001/diff/1/5#newcode14
client/deps/gtest/gtest.py:14: shutil.copyfile(usr_lib + '/libgtest.a', top_dir
+ '/libgtest.a')
On 2010/03/31 19:01:34, sosa wrote:
> Didn't you add the build of gtest to the autotest ebuild rdepend or was it
> already in the sysroot?
> 
> On 2010/03/31 04:34:42, petkov wrote:
> > On 2010/03/31 01:38:56, kmixter1 wrote:
> > > nit: Probably would be easier to maintain to have a list of filenames and
> loop
> > > through copying them
> > 
> > Second that. It seems that in this case you could simply copy libgtest*
> > 
> > I'm a bit confused though... How does gtest get built? It's  either a
> dependence
> > of autotest.ebuild (which it seems it's not), or it's a dependence for
> chromeos
> > or chromeos-dev, in which case it doesn't need a client/deps because it ends
> up
> > on the image anyway...
> > 
> 
> 

Added to the rdepend - but that will be a separate CL (different repo).

http://codereview.chromium.org/1597001/diff/1/5#newcode16
client/deps/gtest/gtest.py:16: shutil.copyfile(usr_lib + '/libgtest.so.0.0.0',
top_dir + '/libgtest.so.0.0.0')
On 2010/03/31 01:38:56, kmixter1 wrote:
> 80 chars here and elsewhere

Done.

http://codereview.chromium.org/1597001/diff/1/7
File client/site_tests/example_unit_test/example_unit_test.py (right):

http://codereview.chromium.org/1597001/diff/1/7#newcode4
client/site_tests/example_unit_test/example_unit_test.py:4: 
On 2010/03/31 19:01:34, sosa wrote:
> You may want to make this into a base class that other unittests inherit from.

> This way you could use setup() and initialize() to do the deps stuff and
cleanup
> to do the copying steps.  This way other unittests could just subclass and not
> have to worry about deps at all.
> 
> See client/bin/site_ui_test as an example

Done.

http://codereview.chromium.org/1597001/diff/1/7#newcode43
client/site_tests/example_unit_test/example_unit_test.py:43:
self.write_perf_keyval({'gtest': result.stdout})
On 2010/03/31 04:34:42, petkov wrote:
> what's result.stdout? is it a number? if not, this doesn't seem to work.

It's the full gtest report - it seems to work fine. A run will show as below. I
could just log it but I thought it would be easier for a reporting mechanism to
do something useful with it if it were in the dictionary.

-----

gtest{perf}=[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ExampleTest
[ RUN      ] ExampleTest.All
[       OK ] ExampleTest.All (1 ms)
[----------] 1 test from ExampleTest (1 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (3 ms total)
[  PASSED  ] 1 test.


gcov{perf}={'main.cc': 100.0}


> 
>

http://codereview.chromium.org/1597001/diff/1/8
File client/site_tests/example_unit_test/src/Makefile (right):

http://codereview.chromium.org/1597001/diff/1/8#newcode15
client/site_tests/example_unit_test/src/Makefile:15: $(CXX) $(CXXFLAGS)
$(OPTIONS) $(LDFLAGS) $(PACKAGES) $(OBJECTS) $(LIBS) -o $(TARGET)
On 2010/03/31 01:38:56, kmixter1 wrote:
> 80 chars and below

Done.

http://codereview.chromium.org/1597001/diff/1/8#newcode25
client/site_tests/example_unit_test/src/Makefile:25: #$(TARGET): $(OBJECTS)
On 2010/03/31 04:34:42, petkov wrote:
> It seems you don't need the commented out code -- remove!

Done.

http://codereview.chromium.org/1597001/diff/1/11
File server/site_tests/example_unit_test_server/example_unit_test_server.py
(right):

http://codereview.chromium.org/1597001/diff/1/11#newcode12
server/site_tests/example_unit_test_server/example_unit_test_server.py:12: def
GenGCovReport(report, files):
On 2010/03/31 04:34:42, petkov wrote:
> per style, i'd change to all lower-case with underscores.
> 

Done.

http://codereview.chromium.org/1597001/diff/1/11#newcode24
server/site_tests/example_unit_test_server/example_unit_test_server.py:24: class
example_unit_test_server(test.test):
On 2010/03/31 01:38:56, kmixter1 wrote:
> Seems like instead of calling this an example, you could pass a parameter to
> this class which is the client unit test to run, and this class would just be
> the unit test framework class.  Could call it something like
framework_Coverage.
>  You would still need to know the list of cc files but possibly this is
> something the client test could always write out in its result directory.

Refactored into a UnitTestServer base class.

http://codereview.chromium.org/1597001/diff/1/11#newcode46
server/site_tests/example_unit_test_server/example_unit_test_server.py:46:
report = subprocess.Popen(['gcov', ''.join(test_files)],
On 2010/03/31 04:34:42, petkov wrote:
> use utils.system instead, it's the autotest way.
> 

Done.

http://codereview.chromium.org/1597001/diff/1/11#newcode61
server/site_tests/example_unit_test_server/example_unit_test_server.py:61:
self.write_perf_keyval({'gcov': filtered})
On 2010/03/31 01:38:56, kmixter1 wrote:
> filtered is a dictionary right?  I didn't think this would work.

Seems to work great.

[petkov, 2010-04-08 21:01:31]

http://codereview.chromium.org/1597001/diff/1012/13001
File client/bin/unit_test.py (right):

http://codereview.chromium.org/1597001/diff/1012/13001#newcode40
client/bin/unit_test.py:40: self.write_perf_keyval({'gtest': result.stdout})
This is not a perf keyval. If you'd like to preserv result.stdout, I would write
it out in a separate file under self.resultsdir -- this will be copied to the
server and can be manipulated later.

http://codereview.chromium.org/1597001/diff/1012/13002
File client/bin/unit_test_server.py (right):

http://codereview.chromium.org/1597001/diff/1012/13002#newcode1
client/bin/unit_test_server.py:1: # Copyright (c) 2009 The Chromium OS Authors.
All rights reserved.
This file should be under server/bin, no?

http://codereview.chromium.org/1597001/diff/1012/13002#newcode9
client/bin/unit_test_server.py:9: from autotest_lib.server import test, autotest
sort

http://codereview.chromium.org/1597001/diff/1012/13002#newcode24
client/bin/unit_test_server.py:24: class UnitTestServer(test.test):
Per CODING_STYLE, use lower case for class names.

http://codereview.chromium.org/1597001/diff/1012/13002#newcode57
client/bin/unit_test_server.py:57: self.write_perf_keyval({'gcov': filtered})
Again, write it out as a separate file under self.resultsdir, through %r for
example. This is not a perf keyval.

http://codereview.chromium.org/1597001/diff/1012/13003
File client/deps/gtest/README (right):

http://codereview.chromium.org/1597001/diff/1012/13003#newcode5
client/deps/gtest/README:5: This deps brings install gtest into an image.  Tests
that depend
"brings install" ?

[Sean Parent, 2010-04-09 01:18:55]

Okay - What we have now is that the perf keyval will contain an entry for each
file showing the coverage. i.e.:

main.cc=100.0
chromos/object/object.h=99.2

The client results directory will contain a file with the same final name with a
.gcov suffix as each of these (hmm.. don't know how to handle conflicts at the
moment) with the coverage information. example-

results/main.cc.gcov
        -:    0:Source:main.cc
        -:    0:Graph:main.gcno
        -:    0:Data:main.gcda
        -:    0:Runs:1
        -:    0:Programs:1
        -:    1:// Copyright (c) 2010 The Chromium OS Authors. All rights
reserved.
        -:    2:// Use of this source code is governed by a BSD-style license
that can be
        -:    3:// found in the LICENSE file.
        -:    4:
        -:    5:#include "gtest/gtest.h"
        -:    6:
        4:    7:TEST(ExampleTest, All) {
        -:    8:  // Test something
        1:    9:  EXPECT_EQ(10, 10);
        1:   10:}
        -:   11:
        1:   12:int main(int argc, char **argv) {
        1:   13:  ::testing::InitGoogleTest(&argc, argv);
        1:   14:  return RUN_ALL_TESTS();
        3:   15:}


Also in the results directory is the gtest results in a file named
unit_test_result.txt which will look something like this:

[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from ExampleTest
[ RUN      ] ExampleTest.All
[       OK ] ExampleTest.All (0 ms)
[----------] 1 test from ExampleTest (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1 ms total)
[  PASSED  ] 1 test.

The basic pass/fail state is a reflection of the gtest result for the test. This
should give us everything accessible through the dashboard (including a graph
for the file coverage). Later we can see if we can't have a better format to
navigate the results.

http://codereview.chromium.org/1597001/diff/1012/13001
File client/bin/unit_test.py (right):

http://codereview.chromium.org/1597001/diff/1012/13001#newcode40
client/bin/unit_test.py:40: self.write_perf_keyval({'gtest': result.stdout})
On 2010/04/08 21:01:31, petkov wrote:
> This is not a perf keyval. If you'd like to preserv result.stdout, I would
write
> it out in a separate file under self.resultsdir -- this will be copied to the
> server and can be manipulated later.
> 
> 

okay - written to unit_test_result.txt

http://codereview.chromium.org/1597001/diff/1012/13002
File client/bin/unit_test_server.py (right):

http://codereview.chromium.org/1597001/diff/1012/13002#newcode1
client/bin/unit_test_server.py:1: # Copyright (c) 2009 The Chromium OS Authors.
All rights reserved.
On 2010/04/08 21:01:31, petkov wrote:
> This file should be under server/bin, no?
> 

Done.

http://codereview.chromium.org/1597001/diff/1012/13002#newcode9
client/bin/unit_test_server.py:9: from autotest_lib.server import test, autotest
On 2010/04/08 21:01:31, petkov wrote:
> sort

Done.

http://codereview.chromium.org/1597001/diff/1012/13002#newcode24
client/bin/unit_test_server.py:24: class UnitTestServer(test.test):
On 2010/04/08 21:01:31, petkov wrote:
> Per CODING_STYLE, use lower case for class names.
> 

Done.

http://codereview.chromium.org/1597001/diff/1012/13002#newcode57
client/bin/unit_test_server.py:57: self.write_perf_keyval({'gcov': filtered})
On 2010/04/08 21:01:31, petkov wrote:
> Again, write it out as a separate file under self.resultsdir, through %r for
> example. This is not a perf keyval.
> 

Done.

http://codereview.chromium.org/1597001/diff/1012/13003
File client/deps/gtest/README (right):

http://codereview.chromium.org/1597001/diff/1012/13003#newcode5
client/deps/gtest/README:5: This deps brings install gtest into an image.  Tests
that depend
On 2010/04/08 21:01:31, petkov wrote:
> "brings install" ?

You got somet'n t' say?

done.

[petkov, 2010-04-09 21:13:31]

A couple of comments, LGTM overall -- it's a good starting point. I'd suggest
you get at least one more review though -- probably from kmixter.

Also, there's some discussion about how to compile and run unittest and... it
seems that it might be best to build unittest as part of the portage build
system as a bi-product of the normal build. The the autotest just needs to run
them. Or something like that -- needs a bit more thinking.

http://codereview.chromium.org/1597001/diff/15002/16001
File client/bin/unit_test.py (right):

http://codereview.chromium.org/1597001/diff/15002/16001#newcode38
client/bin/unit_test.py:38: logging.debug(result.stderr)
Shouldn't you be processing the generated result file, grep for FAIL and signal
a test failure if one of the unittests fails?
Alternatively, you could count the number of pass/fail and generate a perf
keyval with percentage pass count... It might still be nice to FAIL if at least
one unittest fails...

http://codereview.chromium.org/1597001/diff/15002/16001#newcode39
client/bin/unit_test.py:39: logging.info('result: ' + self.resultsdir +
'/unit_test_result.txt')
Now this is something you could write as a test_keyval :-)

[kmixter1, 2010-04-09 22:07:22]

LGTM

Very nice. See inlined comments about the thoughts on unit testing this has
helped start.

http://codereview.chromium.org/1597001/diff/15002/16007
File client/site_tests/example_UnitTest/example_UnitTest.py (right):

http://codereview.chromium.org/1597001/diff/15002/16007#newcode7
client/site_tests/example_UnitTest/example_UnitTest.py:7: class
example_UnitTest(unit_test.unit_test):
I wish we could avoid these tiny files, but I don't know how to.

http://codereview.chromium.org/1597001/diff/15002/16008
File client/site_tests/example_UnitTest/src/Makefile (right):

http://codereview.chromium.org/1597001/diff/15002/16008#newcode14
client/site_tests/example_UnitTest/src/Makefile:14: $(TARGET): $(OBJECTS)
As Darin mentioned, our current thinking on how to do unit testing, which mostly
dovetails well with this CL, is:
a) you add to the ebuild a feature/use flag to indicate you want to build unit
tests
b) it builds and installs those unit tests which must be gtest-harness tests
into %SYSROOT%/usr/local/unittests
c) installing the image (specifically the stateful partition) puts them on the
device
d) an autotest finds them all and runs them and reports their results

we could put both gcov/instrumented gtest executables and regular ones there,
though probably we'd want to just have all unit tests be gcov/instrumented. 
Which means, we could pretty much parameterize your UnitTest classes to take
executable names and have a control file that finds all the tests and runs them
as separate autotest tags.

In any case, that part's not built yet, so this is a good example to start with.

http://codereview.chromium.org/1597001/diff/15002/16010
File server/bin/unit_test_server.py (right):

http://codereview.chromium.org/1597001/diff/15002/16010#newcode20
server/bin/unit_test_server.py:20: results[f] = float(match.group(1))
I was under the impression that the autotest framework does not like keys which
are not valid python identifiers.  For instance a/b or even a\/b as re.escape
would change it.  Certainly the constraint checking code couldn't work with
these.  You probably need to do some more sophisticated escaping to make this
work with all of autotest.  I won't block the change for it, but beware. :)

[Sean Parent, 2010-04-10 02:46:07]

I cleaned up the keyval keys and went ahead and pushed. Now I'll move over a
real unit test just to make sure everything holds together.

http://codereview.chromium.org/1597001/diff/15002/16001
File client/bin/unit_test.py (right):

http://codereview.chromium.org/1597001/diff/15002/16001#newcode38
client/bin/unit_test.py:38: logging.debug(result.stderr)
If one of the unit tests fails then the command will return non-zero, run will
throw an exception, and the test will fail.

On 2010/04/09 21:13:31, petkov wrote:
> Shouldn't you be processing the generated result file, grep for FAIL and
signal
> a test failure if one of the unittests fails?
> Alternatively, you could count the number of pass/fail and generate a perf
> keyval with percentage pass count... It might still be nice to FAIL if at
least
> one unittest fails...
>

http://codereview.chromium.org/1597001/diff/15002/16001#newcode39
client/bin/unit_test.py:39: logging.info('result: ' + self.resultsdir +
'/unit_test_result.txt')
On 2010/04/09 21:13:31, petkov wrote:
> Now this is something you could write as a test_keyval :-)
> 

Since this is client side - I didn't think it would be that useful, easy enough
to navigate to from the final directory from the server test.

http://codereview.chromium.org/1597001/diff/15002/16010
File server/bin/unit_test_server.py (right):

http://codereview.chromium.org/1597001/diff/15002/16010#newcode20
server/bin/unit_test_server.py:20: results[f] = float(match.group(1))
On 2010/04/09 22:07:23, kmixter1 wrote:
> I was under the impression that the autotest framework does not like keys
which
> are not valid python identifiers.  For instance a/b or even a\/b as re.escape
> would change it.  Certainly the constraint checking code couldn't work with
> these.  You probably need to do some more sophisticated escaping to make this
> work with all of autotest.  I won't block the change for it, but beware. :)

I replaced all "." and "/" characters with "_". That should keep autotest happy.

[petkov, 2010-04-10 04:27:51]

> On 2010/04/09 22:07:23, kmixter1 wrote:
> > I was under the impression that the autotest framework does not like keys
> which
> > are not valid python identifiers.  For instance a/b or even a\/b as
re.escape
> > would change it.  Certainly the constraint checking code couldn't work with
> > these.  You probably need to do some more sophisticated escaping to make
this
> > work with all of autotest.  I won't block the change for it, but beware. :)
> 
> I replaced all "." and "/" characters with "_". That should keep autotest
happy.

Just to clarify things a bit -- you can use pretty much any key name as long as
it doesn't contain '=', if you don't intend to set constraints based on the
keyvals.

If you'd like to convert paths to proper identifiers, replacing '.' and '/' with
'_' would certainly not be enough.