Merge lp://qastaging/~nelson-chu/opencompute/add-ocp-cpu-memory-job into lp://qastaging/opencompute/checkbox
Proposed by
Nelson Chu
Status: | Superseded |
---|---|
Proposed branch: | lp://qastaging/~nelson-chu/opencompute/add-ocp-cpu-memory-job |
Merge into: | lp://qastaging/opencompute/checkbox |
Diff against target: |
506 lines (+365/-43) 7 files modified
data/whitelists/opencompute-certify-local.whitelist (+0/-43) debian/changelog (+19/-0) jobs/TC-001-0001-CPU_Memory.txt.in (+31/-0) jobs/local.txt.in (+7/-0) scripts/cpu_info (+100/-0) scripts/memory_info (+78/-0) scripts/processor_topology (+130/-0) |
To merge this branch: | bzr merge lp://qastaging/~nelson-chu/opencompute/add-ocp-cpu-memory-job |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeff Lane | Pending | ||
Nelson Chu | Pending | ||
Review via email: mp+206489@code.qastaging.launchpad.net |
This proposal supersedes a proposal from 2014-02-13.
This proposal has been superseded by a proposal from 2014-02-18.
Description of the change
Scripts and annotations have been modified.
To post a comment you must log in.
Hi Nelson,
Thank you for breaking this into smaller pieces... it makes review a LOT easier. Now, there are some things that need fixing.
I'll take them one file at a time: /opencompute- certify- local.whitelist looks fine.
data/whitelists
jobs/TC- 001-0001- CPU_Memory. txt.in: 0001-003- Memory_ Information also needs 'user: root' added to properly run.
1: any job that calls a script that needs root permissions to run must include the 'user: root' definition. See the file 'jobs/cpu.txt.in' for some examples where this is necessary. When I run the script cpu_info without root, the cache data is not returned and the test will fail. So this job needs 'user:root'
2: TC-001-
jobs/local.txt.in
1: The word "Verified" should be "Verify" in the description field.
scripts/cpu_info: 0001-001- CPU_information with and without root permissions, I get the following different outputs: klaatu: ~/development/ ocp-nelson- memory- cpu-test$ ./scripts/ cpu_infoIntel( R) Core(TM) i7 CPU Q 720 @ 1.60GHz klaatu: ~/development/ ocp-nelson- memory- cpu-test$ echo $? klaatu: ~/development/ ocp-nelson- memory- cpu-test$ sudo ./scripts/cpu_info klaatu: ~/development/ ocp-nelson- memory- cpu-test$ echo $?
1: When I manually run the cpu_info script for the test TC-001-
bladernr@
Can not found any CPU cache.
bladernr@
30
bladernr@
Intel(R) Core(TM) i7 CPU Q 720 @ 1.60GHz
L1 Cache 32 KB
L2 Cache 256 KB
L3 Cache 6 MB
L3 cache size less than 20MB.
bladernr@
50
I don't have a system that meets the test criteria, so it will always fail for me. First, the explanation for failure should be more explicit. Example: 'FAIL: Can not find any CPU cache.' for the first example. This is more important in the second example where 'L3 cache size less than 20MB' looks like part of the lshw output. It would be better more explicitly stated as 'FAIL: L3 cache size less than 20MB.'
2: You don't need all those explicit error codes. Checkbox only knows and stores 0 and Not 0 exit codes. A 0 exit code indicates a test passed. A non-zero exit code indicates a failure. Checkbox does not store the actual exit codes. This is not necessarily something you need to change, but you DO need to be aware of the behaviour in case a script behaves differently than expected when you run it via checkbox.
3: The description and spec for this test says: "CPU model should belong to Intel Xeon processor E5-2600 family..." but your test does not fail on non-Xeon processors. For example, when I comment out the error code return for my cache limit on my laptop, I get this output: klaatu: ~/development/ ocp-nelson- memory- cpu-test$ sudo ./scripts/cpu_info; echo $?
bladernr@
Intel(R) Core(TM) i7 CPU Q 720 @ 1.60GHz
L1 Cache 32 KB
L2 Cache 256 KB
L3 Cache 6 MB
L3 cache size less than 20MB.
0
but my laptop should clearly fail the test case since it's not up to OCP spec.
4: The output needs to be cleaned up a bit. Sorry, I know English is a second language for you, so I'll try to help as much as I can.
"Can not parser" should be "Can not parse"
"Can not found" should be "Can not find"
"# Parser lshw XML for gather" should be "# Parse lshw XML for gathering"
scripts/ memory_ info:
1: Needs to be run as ...