Collect dmesg when running tests #823
Conversation
douglas-raillard-arm
force-pushed
the
_pr101
branch
from
5386ffa to
dc76a42
Compare
qais-yousef
left a comment
qais-yousef
left a comment
There was a problem hiding this comment.
I have a cuople of questions but it looks good to me in general. Dmesg is useful to have for sure.
|
|
||
| self.dmesg_out = self.target.execute(cmd) | ||
|
|
||
| def get_trace(self, outfile): |
There was a problem hiding this comment.
can this be called get_log() instead?
There was a problem hiding this comment.
That was named to match the "abstract" method defined in the base class (TraceCollector), but it would indeed make more sense as get_log(). Can you post that comment on devlib's PR as well, so we can have the opinions of the maintainers there ?
ARM-software/devlib#376
| ) | ||
|
|
||
|
|
||
| class DmesgCollector(TraceCollector): |
There was a problem hiding this comment.
This inheritance relationship doesn't sound right to me. From naming point of view at least. What facilities in the parent class you care about here?
If it is to auto start/stop then it seems to me TraceCollector and DmesgCollector should have a common parent class.
e.g
BufferCollector
|
+------------- +----------------+
| |
TraceCollector DmesgCollector
There was a problem hiding this comment.
There are already a number of other collectors in devlib like LogcatCollector, which already inherit from TraceCollector. That hierarchy of names probably needs to be preserved for backward compatibilities, but if you think it's worth a rename, having aliases is possible if devlib maintainers are happy with that.
There was a problem hiding this comment.
What facilities in the parent class you care about here?
Mostly the context manager (__enter__/__exit__) and the "promise on the API" (all collectors can be expected to behave roughly the same way without looking at each specific case). I initially tried to implement it as an instrument, but it was not suited at all. TraceCollector seemed to have a number of derived classes doing more or less the same thing as I do.
8512f116 trace: Add DmesgCollector be8b87d5 module/sched: Fix/simplify procfs packing behaviour d76c2d63 module/sched: Make get_capacities() work with hotplugged CPUs 8bfa0502 module/sched: SchedProcFSData: Don't assume SD name is always present git-subtree-dir: external/devlib git-subtree-split: 8512f116fc5de214fd73cacf474b3109323a5754
This gives a number of utilities, among them the real "dmesg" that supports more options than the BusyBox one.
Make sure the same name is used everywhere for trace and dmesg log files in the res_dir.
douglas-raillard-arm
force-pushed
the
_pr101
branch
from
dc76a42 to
c77e728
Compare
3 participants
EDIT: ready to merge, devlib's PR has been merged