----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3461/#review8277 -----------------------------------------------------------
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). tests/testing/helpers.py (line 108) <http://reviews.gem5.org/r/3461/#comment7083> Can you add a comment description here that describes that this is an example test for the ProcessHelper? tests/testing/tests.py (line 49) <http://reviews.gem5.org/r/3461/#comment7084> 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? tests/tests.py (line 93) <http://reviews.gem5.org/r/3461/#comment7085> I think this abstract filter is going to be very useful... Unfortunately, I tried running this script and changing this (these) argument, but I'm still not sure I understand how it works. Please make sure to add notes about this argument in the comments. tests/tests.py (line 191) <http://reviews.gem5.org/r/3461/#comment7082> Can you add a description here that is similar to the patch description? I played around with this patch, and there were some things I couldn't figure out just from the help messages (e.g. see note above). - Joel Hestness On April 28, 2016, 3: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, 3: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
