tuned icon indicating copy to clipboard operation
tuned copied to clipboard

[WIP] Make Tuned unit-testable

Open olysonek opened this issue 5 years ago • 23 comments

Please do not merge this yet. It's only a request for comments and still work-in-progress at this point.

The commit "[WIP] Add a new file access API" adds a new API. Its commit message describes the motivation for it and its advantages, and also includes some open questions (these are not that important to get right however, as this is internal API). The other commits make use of the new API.

@yarda @TomasKorbar What do you guys think? Is it ok? Can we make it better?

olysonek avatar Apr 07 '19 08:04 olysonek

Rebased on top of origin/master.

olysonek avatar Feb 19 '20 16:02 olysonek

There was an error while running a copr build:

maximum recursion depth exceeded

You can re-trigger build by adding a comment (/packit copr-build) into this pull request.

This pull request introduces 3 alerts when merging 73a4c0e63ea14156058c0bd05895cacea9fc8c18 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Use of the return value of a procedure

lgtm-com[bot] avatar Feb 19 '20 16:02 lgtm-com[bot]

/packit copr-build

olysonek avatar Feb 19 '20 16:02 olysonek

There was an error while running a copr build:

maximum recursion depth exceeded while calling a Python object

You can re-trigger build by adding a comment (/packit copr-build) into this pull request.

GitHub had outage an hour ago so I hope this is not the aftermath. I was checking our monitoring and couldn't find an issue related to the recursion problem.

TomasTomecek avatar Feb 19 '20 16:02 TomasTomecek

The latest changes move all plugins to their own directories, which allows us to create private libraries for plugins. This allows us to much more easily test certain functionality. An example of this is in the last commit "WIP: cpu: Move functions that access files to a private library".

olysonek avatar Feb 20 '20 16:02 olysonek

@yarda This is now in a proof-of-concept state. I'd appreciate your opinion on this.

olysonek avatar Feb 20 '20 16:02 olysonek

This pull request introduces 23 alerts and fixes 22 when merging 2a93a42954c5dc44296ea345943107fc41a86391 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 22 for Unused import
  • 1 for Use of the return value of a procedure

fixed alerts:

  • 22 for Unused import

lgtm-com[bot] avatar Feb 20 '20 16:02 lgtm-com[bot]

Congratulations! One of the builds has completed. :champagne:

You can install the built RPMs by following these steps:

  • sudo yum install -y dnf-plugins-core on RHEL 8
  • sudo dnf install -y dnf-plugins-core on Fedora
  • dnf copr enable packit/redhat-performance-tuned-176
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

This pull request introduces 3 alerts and fixes 42 when merging 895370ad13d4b6e00b55c07cf4814c1b31224621 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 20 '20 17:02 lgtm-com[bot]

It's still work-in-progress though. Please don't merge :).

olysonek avatar Feb 20 '20 18:02 olysonek

This pull request introduces 1 alert and fixes 42 when merging 1bfd59915ad13f09511db2d8d07abebcdb087498 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 21 '20 11:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging 630fb0acd586c2f45f32c8fcabc139fab71ed949 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 21 '20 14:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging fc2d199b2a477ffe0e8f69e72e4f0d143e07f813 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 21 '20 15:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging 735d2069186a4ce9395f959a39d5b4b8810ecc5c into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 21 '20 16:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging c37542dae0b8d22b989a26386e6e1363d0ec7ebb into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 24 '20 13:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging b8e68375b601ad188cf07831d457e930d5c4eab3 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 24 '20 17:02 lgtm-com[bot]

This allows us to write tests for some bug fixes in the form of unit tests. See commits sysctl: Test that non-existent settings are ignored in system configs and sysctl: Test that reapply_sysctl ignores configs from /usr

olysonek avatar Feb 26 '20 16:02 olysonek

This pull request introduces 1 alert and fixes 42 when merging 41907481418fd77a9aade5add49d940cbcd53b18 into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 26 '20 16:02 lgtm-com[bot]

Note: for simplicity, all the methods in the library of plugin_cpu are currently in a single class CPULatencyLibrary. We might want to split it up into e.g. IntelPStateManager and SamplingDownFactorManager.

olysonek avatar Feb 27 '20 12:02 olysonek

This pull request introduces 1 alert and fixes 42 when merging 91e5919b7875538cb5f024a76388df91c5dabc1f into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 27 '20 15:02 lgtm-com[bot]

This pull request introduces 1 alert and fixes 42 when merging 91e5919b7875538cb5f024a76388df91c5dabc1f into 9bb73e8f407ae3ce17031e020ab112ac8c8cef45 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 42 for Unused import

lgtm-com[bot] avatar Feb 27 '20 15:02 lgtm-com[bot]