Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Precooked tests for monitors #195

Merged
merged 1 commit into from
Aug 25, 2021
Merged

Conversation

quantum-shift
Copy link
Collaborator

@quantum-shift quantum-shift commented Jul 27, 2021

Monitors have been adapted to optionally read from precooked sources. Monotonicity tests have been added for cpumon, iomon, netmon and memmon. A value test has been added for nvidiamon.

Changes:

  • The update_stats() methods of the abstract Imonitor base class and all of its derived classes have been modified to accept an optional argument read_path that points to the directory the monitor should read from.
  • Auxiliary methods have been added to netmon and nvidiamon classes for reading from precooked sources to preserve the structure of the update_stats() method of these classes since both these classes use separate functions to procure the data.

Source files added:

  • scripts/precook_tests.py: This generates the scripts/precooked_tests directory that contains the precooked test data.
  • tests/test_values.cpp: This is a test harness containing the new tests. GoogleTest framework is used for the tests. Presently, it contains monotonicity and value tests for cpumon, iomon, netmon, memmon and nvidiamon.

Copy link
Member

@graeme-a-stewart graeme-a-stewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @quantum-shift - nice work!

I have made quite a few suggestions to improve the Python script and also to make nvidiamon::update_stats simpler with getline.

package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Show resolved Hide resolved
package/src/iomon.cpp Outdated Show resolved Hide resolved
package/src/iomon.cpp Outdated Show resolved Hide resolved
package/src/nvidiamon.cpp Outdated Show resolved Hide resolved
package/tests/test_values.cpp Outdated Show resolved Hide resolved
@quantum-shift quantum-shift force-pushed the precook-tests branch 2 times, most recently from f835a66 to 5dbe4ee Compare July 31, 2021 12:07
@quantum-shift
Copy link
Collaborator Author

Thank you for you detailed comments, @graeme-a-stewart. I have made the changes.

@graeme-a-stewart
Copy link
Member

Hi @quantum-shift

So, I am seeing a test failure:

[ RUN      ] CpumonTest.CpumonMonitonicityTestFixed
/home/graemes/code/prmon/package/tests/test_values.cpp:72: Failure
Expected equality of these values:
  stat.second
    Which is: 40000
  (10000000 * 2) / sysconf(_SC_CLK_TCK)
    Which is: 200000
[  FAILED  ] CpumonTest.CpumonMonitonicityTestFixed (1 ms)

And in main it definitely doesn't fail.

It also looks like a tests gets dropped - there are only 19 run instead of 20, with all of yours there, but none of the tests I added in #196.

Can you check? It looks like there was a merge in tests/CMakeLists.txt that dropped my tests.

@quantum-shift
Copy link
Collaborator Author

Hi @graeme-a-stewart. That is quite strange. The tests added in #196 seem to have run in the CI checks. I tested locally and the tests run there as well.

Could you tell me if the cpumon test is the only one failing among the newly added tests?

@graeme-a-stewart
Copy link
Member

graemes@Surface-Graeme:~/build/prmon$ cmake --build .
[0/1] Re-running CMake...
-- Setting Python test binary to 'python3' (use -DPYTHON_TEST to change)
-- Setting clang-format test binary to 'clang-format' (use -DCLANG_FORMAT to change)
-- Configuring done
-- Generating done
-- Build files have been written to: /home/graemes/build/prmon
[22/22] Linking CXX executable package/tests/test_values
graemes@Surface-Graeme:~/build/prmon$ cmake --build . --target test
[0/1] Running tests...
Test project /home/graemes/build/prmon
      Start  1: IomonTest.IomonMonitonicityTestFixed
 1/19 Test  #1: IomonTest.IomonMonitonicityTestFixed .....   Passed    0.01 sec
      Start  2: CpumonTest.CpumonMonitonicityTestFixed
 2/19 Test  #2: CpumonTest.CpumonMonitonicityTestFixed ...***Failed    0.01 sec
      Start  3: MemmonTest.MemmonValueTestFixed
 3/19 Test  #3: MemmonTest.MemmonValueTestFixed ..........   Passed    0.01 sec
      Start  4: NetmonTest.NetmonMonitonicityTestFixed
 4/19 Test  #4: NetmonTest.NetmonMonitonicityTestFixed ...   Passed    0.02 sec
      Start  5: NvidiamonTest.NvidiamonValueTestFixed
 5/19 Test  #5: NvidiamonTest.NvidiamonValueTestFixed ....   Passed    0.10 sec
      Start  6: testCPUsingle
 6/19 Test  #6: testCPUsingle ............................   Passed   10.71 sec
      Start  7: testCPUmultithread
 7/19 Test  #7: testCPUmultithread .......................   Passed   10.16 sec
      Start  8: testCPUmultiproc
 8/19 Test  #8: testCPUmultiproc .........................   Passed   12.15 sec
      Start  9: testCPUinvoke
 9/19 Test  #9: testCPUinvoke ............................   Passed   10.14 sec
      Start 10: basicIOsingle
10/19 Test #10: basicIOsingle ............................   Passed    6.74 sec
      Start 11: basicIOmultithread
11/19 Test #11: basicIOmultithread .......................   Passed    6.79 sec
      Start 12: basicIOmultiproc
12/19 Test #12: basicIOmultiproc .........................   Passed    6.74 sec
      Start 13: basicNET
13/19 Test #13: basicNET .................................   Passed    7.96 sec
      Start 14: singleMem
14/19 Test #14: singleMem ................................   Passed   10.15 sec
      Start 15: childMem
15/19 Test #15: childMem .................................   Passed   10.24 sec
      Start 16: basicCOUNT
16/19 Test #16: basicCOUNT ...............................   Passed   10.17 sec
      Start 17: testUnits
17/19 Test #17: testUnits ................................   Passed    3.18 sec
      Start 18: testExitCode
18/19 Test #18: testExitCode .............................   Passed    3.13 sec
      Start 19: testDisable
19/19 Test #19: testDisable ..............................   Passed    6.19 sec

95% tests passed, 1 tests failed out of 19

Total Test time (real) = 114.63 sec

The following tests FAILED:
          2 - CpumonTest.CpumonMonitonicityTestFixed (Failed)
Errors while running CTest
FAILED: CMakeFiles/test.util
cd /home/graemes/build/prmon && /usr/bin/ctest --force-new-ctest-process
ninja: build stopped: subcommand failed.

@graeme-a-stewart
Copy link
Member

Oh, wait - I maybe didn't fetch the forced update properly first... hold on, testing again

@graeme-a-stewart
Copy link
Member

Ah yes, this was my bad - I thought I had fetched from your fork, but I guess I didn't.

All tests are running and passing now.

Sorry for the noise!

Copy link
Member

@graeme-a-stewart graeme-a-stewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @quantum-shift - all looks pretty good. Just a few last comments on the Python script.

package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Outdated Show resolved Hide resolved
package/scripts/precook_test.py Show resolved Hide resolved
@quantum-shift
Copy link
Collaborator Author

Hi @graeme-a-stewart. I have updated the python script. I have also added documentation about adding precooked tests and modified the documentation on adding monitors to account for the new read_path argument.

scripts/precook_test.py is the generator of scripts/precooked_tests directory

test_values.cpp uses GoogleTest library to test values of individual monitors

tests/CMakeLists.cpp updated to include new tests

Added documentation for precooked tests

Added command line options to precook_test.py
@graeme-a-stewart
Copy link
Member

Excellent work @quantum-shift - we can merge this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants