> On May 2, 2016, 10:01 p.m., Joel Hestness wrote: > > Overall, this looks pretty solid - much more like a "test harness" than the > > existing regressions. Thanks! > > > > Given that this change is already pretty significant, are there other > > changes we should introduce or plan to make? For example, below I suggest > > clarifying test naming conventions. > > > > Also, I can see how this change might make it easier for users to add and > > run their own tests, but only within the existing cumbersome tests > > naming/directory structure. Can we make it a little clearer how to add > > tests with a different naming/directory structure? Ultimately, something > > I'd really like is the ability to add a named group of tests that validate > > subsets of functionality (e.g. current "regressions" are such a group, or > > I'd like to have a group of tests for both cache coherence and memory > > consistency for different ISAs, CPU and/or GPU, and different Ruby > > coherence protocols).
Thanks for a very amibtious review! :) I like your ideas of cleaning up the test naming. It definitely makes sense and is something I'd like to do at some point. We also need to sit down and about what we test and how (e.g., not being able to redistribute test binaries is a big issue), but that is an orthogonal question which I'd like to discuss on the dev list in the not too distant future. As you alread noticed it's already quite a large piece of code, so I'd like to avoid making more changes to the tests (like reorganising the test strucutre) in this commit. One of the goals when writing this was that after applying RB3462, there should be no difference to most users of the regression system. > On May 2, 2016, 10:01 p.m., Joel Hestness wrote: > > tests/testing/tests.py, line 49 > > <http://reviews.gem5.org/r/3461/diff/1/?file=55253#file55253line49> > > > > In general, I've found the tests directory naming structure to be > > confusing and underdocumented. Can we add some clear comments here and make > > these test character names more precise? > > > > For instance, "category" is either "quick" or "long"; it's confusing to > > call these test categories, since they just appear to describe the duration > > of the test. Maybe "duration" instead? Can you document roughly what > > "quick" and "long" should be? > > > > The "mode" name is ambiguous, since gem5 code uses "mode" to describe > > numerous different things (e.g. full-system vs. syscall emulation mode, > > memory modes, system/user modes, etc.). Maybe "syscall mode" instead? > > > > I've always found it strange to just use "benchmark" as well, since > > many tests come from different suites... I'm not sure what would be better > > here. > > > > Finally, the name "config" should reflect that it sufficiently > > describes the simulated system. In general, it includes a platform name > > (e.g. tsunami for ALPHA or pc for X86), whether there are multiple systems, > > the type of the CPU cores, and the memory mode. Is there a reason that we > > separate out the ISA or even the OS? Maybe "system config" instead? Can you > > add comments with this detail? I'd like to avoid changing this for now. As you already noted, this is already quite a large change. The existing test infrastructure (e.g., run.py) currently uses the same terminology for the contents of the test tuple. I don't like the way the tuple is structured, but I'd like to save that for a different patch. However, I did change the named tuple to use 'workload' instead of 'benchmark' as that's much more descriptive. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3461/#review8277 ----------------------------------------------------------- On April 28, 2016, 4:20 p.m., Andreas Sandberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3461/ > ----------------------------------------------------------- > > (Updated April 28, 2016, 4:20 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11472:213bea6414bc > --------------------------- > tests: Add test infrastructure as a Python module > > Implement gem5's test infrastructure as a Python module and a run > script that can be used without scons. The new implementation has > several features that were lacking from the previous test > infrastructure such as support for multiple output formats, automatic > runtime tracking, and better support for being run in a cluster > environment. > > Tests consist of one or more steps (TestUnit). Units are run in two > stages, the first a run stage and then a verify stage. Units in the > verify stage are automatically skipped if any unit run stage wasn't > run. The library currently contains TestUnit implementations that run > gem5, diff stat files, and diff output files. > > Existing tests are implemented by the ClassicTest class and "just > work". New tests can that don't rely on the old "run gem5 once and > diff output" strategy can be implemented by subclassing the Test base > class or ClassicTest. > > Test results can be output in multiple formats. The module currently > supports JUnit, text (short and verbose), and Python's pickle > format. JUnit output allows CI systems to automatically get more > information about test failures. The pickled output contains all state > necessary to reconstruct a tests results object and is mainly intended > for the build system and CI systems. > > Since many JUnit parsers parsers assume that test suite names look > like Java package names. We currently output path-like names with > slashes separating components. Test names are translated according to > these rules: > > * '.' -> '-" > * '/' -> '.' > > The test tool, tests.py, supports the following features: > > * Test listing. Example: ./tests.py list arm/quick > > * Running tests. Example: > ./tests.py run -o output.pickle --format pickle \ > ../build/ARM/gem5.opt \ > quick/se/00.hello/arm/linux/simple-timing > > * Displaying pickled results. Example: > ./tests.py show --format summary *.pickle > > Change-Id: I527164bd791237aacfc65e7d7c0b67b695c5d17c > Signed-off-by: Andreas Sandberg <[email protected]> > Reviewed-by: Curtis Dunham <[email protected]> > > > Diffs > ----- > > tests/testing/units.py PRE-CREATION > tests/tests.py PRE-CREATION > tests/testing/__init__.py PRE-CREATION > tests/testing/helpers.py PRE-CREATION > tests/testing/results.py PRE-CREATION > tests/testing/tests.py PRE-CREATION > > Diff: http://reviews.gem5.org/r/3461/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Sandberg > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
