HI Wookey,
On 16 March 2018 at 18:54, Wookey wookey@wookware.org wrote:
On 2018-03-08 05:18 +0000, Wookey wrote:
I'll post my issues/changes here for discussion so we can decide what is upstreamable.
TESTS
it's very easy to build the tests: make -f $(CURDIR)/decoder/build/linux/makefile tests
but I'm not sure what, if anything, to do with them. I see no machinery for running the tests, just an explanation of how to do it by hand.
The tests as they stand are largely manual at present.
Is there any point packaging the test programs (are they useful at development time?) or are they purely for testing that the code works?
The test programs can be used at development time - but the trc_pkt_lister is also a handy utility to examine captured trace. Snapshots can be generated by other tools. The coresight access library(CSAL) - also on github - is one example.
c_api_pkt_print test was developed purely as a check that the C-API interfaces worked - so is less sophisticated in terms of command line options - hence the hard-coded paths you found.
If the latter then they should ideally be run at build-time to check that the built code does indeed do what was intended.
Alternatively if they are not really designed as build-time tests, then they can be run as 'autopackage tests'. This is a (debian-based distro) mechanism for running tests on the installed package, like building a test program against it.
I'm not quite sure how best to proceed here.
Firstly, we can just uploaded the package as-is without worrying about the tests. Testing is optional (but if we run them at build-time they will need to pass on all arches).
Secondly for the existing test to be useful as build-time tests we'd need to do something to check the output against the expected output. Not sure how easy that is (does it change on different arches at all?)
I am working on some updates to create a library self test mechanism that is not reliant on the previously captured snapshot data. That work is probably a couple of months off at best. Once this is ready then the tests will be suitable for run after build.
It seems that the current test stuff sets up a local test environment with local copies of the library. It might make more sense to run these as autopackage tests, which run these tests on an installed copy of the library, but I think that means including the test programs in the package. I'm not sure about that as I've never actually set up an autopackage test - I think you can arrange to use to use build-time artifacts and source artificats as well as packaged artifacts, but I'll need to check.
That is really a result of the project initially not having a make install - plus it ensures that debug tests and libraries are run together when gdb/ddd is used to step through code when debugging.
Log/ppl files are generated by the test programs, but I don't see 'correct' test-case copies of those files in the source. Presumably a pair of thesefor each snapshot would work, assuming that they don't vary too much with architecture/date/whatever? (The path variance noted below could be a pain).
At one point it was intended to have some "correct" versions of the logs - the manual testing is currently to cast an experienced eye over the first couple of pages of data, and initially to manually compare with trace dumps from another decode - e..g the one in ARM's debug tool DS-5. Otherwise I can also check out a previous release to generate a before and after.
The problem with the dumps, is that the amount of data makes it impossible to check every line, so while I believe the decode is all correct, we have no real way of double checking this - hence the change of direction to create self tests where known packets will be generated to push though the library and then check for the results.
It does feel like we should do something with the tests as the great bulk of the package source is lots of snapshot files for testing. it's currently 25MB of test files and 2MB of package (unpacked).
Running ./c_api_pkt_print_test fails, because the hardcoded path goes one level too far back up: decoder/tests/bin/builddir$ ./c_api_pkt_print_test Unable to open file ../../../snapshots/juno_r1_1/cstrace.bin to process trace data
Running it from a temp directory one layer down works:
(Ah yes, this used to be correct, (and I guess remains so upstream) because of your two-level platform build-dir naming <arch-bits>/{rel,dbg})
decoder/tests/bin/builddir/tmp/$ ../c_api_pkt_print_test Library Version 0.8.0
../c_api_pkt_print_test
Idx:86; I_NOT_SYNC : I Stream not synchronised Idx:1650; I_ASYNC : Alignment Synchronisation. Idx:1662; I_TRACE_INFO : Trace Info.; INFO=0x0 ...
This makes it work: Index: libopencsd-0.8.0/decoder/tests/source/c_api_pkt_print_test.c =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/source/c_api_pkt_print_test.c +++ libopencsd-0.8.0/decoder/tests/source/c_api_pkt_print_test.c @@ -63,11 +63,11 @@
/* path to test snapshots, relative to tests/bin/<plat>/<dbg|rel> build output dir */ #ifdef _WIN32 -const char *default_path_to_snapshot = "..\..\..\snapshots\juno_r1_1\"; -const char *tc2_snapshot = "..\..\..\snapshots\TC2\"; +const char *default_path_to_snapshot = "..\..\snapshots\juno_r1_1\"; +const char *tc2_snapshot = "..\..\snapshots\TC2\"; #else -const char *default_path_to_snapshot = "../../../snapshots/juno_r1_1/"; -const char *tc2_snapshot = "../../../snapshots/TC2/"; +const char *default_path_to_snapshot = "../../snapshots/juno_r1_1/"; +const char *tc2_snapshot = "../../snapshots/TC2/"; #endif
/* trace data and memory file dump names and values - taken from snapshot metadata */
Similarly, adjusting the example in the docs for both linux path separators and relative depth, this works: ./trc_pkt_lister -ss_dir ../../snapshots/TC2 -o_raw_unpacked
So, what do you think is best to do about this from an
a) upstream point of view (build-time tests generally a good idea) b) distro point of view (seems like installed-package autopackagtsts might make more sense here). We can of course have both.
Currently I am inclined (for the Debian package) to include that path fix, and adjust the docs correspondingly, and build the tests, but will not wait for a run/check-the-tests implementation of any sort before uploading an initial package to debian. If we decide to do one it can be included in due course.
I'd be fine with this. I would also be fine with only including the snapshot that is directly referenced in the C-API program. The other snapshots will always be available on Github.
Mike
Wookey
Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight