feat: add steps for ram and cpu use #230
Conversation
patrzhan
force-pushed
the
ram-cpu-steps
branch
from
5e87e13 to
4047f51
Compare
|
Unit Tests Coverage Report for aws-greengrass-testing-features
Minimum allowed coverage is Generated by 🐒 cobertura-action against a8433b3 |
|
Unit Tests Coverage Report for aws-greengrass-testing-launcher
Minimum allowed coverage is Generated by 🐒 cobertura-action against a8433b3 |
|
Unit Tests Coverage Report for aws-greengrass-testing-platform-api
Minimum allowed coverage is Generated by 🐒 cobertura-action against a8433b3 |
|
Unit Tests Coverage Report for aws-greengrass-testing-platform-pillbox
Minimum allowed coverage is Generated by 🐒 cobertura-action against a8433b3 |
...rass-testing-features-api/src/main/java/com/aws/greengrass/testing/features/SystemSteps.java
Outdated
Show resolved
Hide resolved
...rass-testing-features-api/src/main/java/com/aws/greengrass/testing/features/SystemSteps.java
Outdated
Show resolved
Hide resolved
Works on MacOS, Linux, and Windows. |
|
|
||
|
|
||
| @ScenarioScoped | ||
| public class SystemSteps { |
There was a problem hiding this comment.
Nit: Can we rename this to something like SystemMetricSteps ?
| private SystemInfo si = new SystemInfo(); | ||
| private HardwareAbstractionLayer hal = si.getHardware(); | ||
| private CentralProcessor cpu = hal.getProcessor(); | ||
| private GlobalMemory ram = hal.getMemory(); |
There was a problem hiding this comment.
Initialize these in a constructor.
| public void recordCpuOrRam(String stat) throws IllegalArgumentException { | ||
| switch (stat) { | ||
| case "CPU": | ||
| double cpuLoad = cpu.getSystemCpuLoad(500) * 100; |
There was a problem hiding this comment.
Milliseconds to wait as the time period we check the cpu load over
There was a problem hiding this comment.
Suggestion: This is good. But, can we have a step that stores the recorded value in scenario context? This will help us test the difference between any two values instead of just the last two. Eg. I record the device's {word} usage statistic as {word} and the test would look the following.
When I record the device's RAM usage statistic as RAM_1
And I wait 10 seconds
When I record the device's RAM usage statistic as RAM_2
And I wait 10 seconds
When I record the device's RAM usage statistic as RAM_3
Then the difference between RAM_1 and RAM_3 is less than 1024 MB
There was a problem hiding this comment.
I like that, replaced the old steps with these in latest commit.
| switch (stat) { | ||
| case "CPU": | ||
| if (cpuList.size() < 2) { | ||
| throw new Exception("Need at least two CPU samples first."); |
There was a problem hiding this comment.
The exception is not very intuitive, it looks like that CPU or RAM usage is recorded implicitly and do not have enough data to compare instead of deliberately calling previous step to do so.
Same in line 77.
There was a problem hiding this comment.
Removed this exception since refactored the steps again, now the records will be saved to scenario context.
| if (ramList.size() < 2) { | ||
| throw new Exception("Need at least two RAM samples first."); | ||
| } | ||
| double ramDiff = ramList.get(0) - ramList.get(1); |
| if (cpuList.size() < 2) { | ||
| throw new Exception("Need at least two CPU samples first."); | ||
| } | ||
| double cpuDiff = cpuList.get(0) - cpuList.get(1); |
There was a problem hiding this comment.
Hard coding the index is not good practice. What if the list has more than 2 records then last and second last index will not be 1 and 0.
There was a problem hiding this comment.
This is also removed since we're now using scenario context, but in this context I was adding newest records to index 0 so that 0 and 1 would always be the latest two.
| } | ||
| double cpuSample2 = Double.parseDouble(scenarioContext.get(statKey2)); | ||
| double cpuSample1 = Double.parseDouble(scenarioContext.get(statKey1)); | ||
| double cpuDiff = cpuSample2 - cpuSample1; |
There was a problem hiding this comment.
Should this be absolute if we're checking only the difference? This is assuming that the latter record is always greater than former one. If that's the case, maybe we should reword the step to reflect that.
There was a problem hiding this comment.
I will update the step wording to reflect that we only care about the increase amount from the first to second sample
There was a problem hiding this comment.
Rename the step to one of these as discussed offline:
Then the CPU usage <metric1> is greater than <metric2> by atmost <threshold> <units>
Then the CPU usage <metric1> is no greater than <metric2> by <threshold> <units>
Then the increase in the CPU usage from <metric1> to <metric2> is less than <threshold> <units>
There was a problem hiding this comment.
Updated in latest commit, also added changes to logging.
| } | ||
| double cpuSample2 = Double.parseDouble(scenarioContext.get(statKey2)); | ||
| double cpuSample1 = Double.parseDouble(scenarioContext.get(statKey1)); | ||
| double cpuDiff = cpuSample2 - cpuSample1; |
| } | ||
| double ramSample2 = Double.parseDouble(scenarioContext.get(statKey2)); | ||
| double ramSample1 = Double.parseDouble(scenarioContext.get(statKey1)); | ||
| double ramDiff = ramSample2 - ramSample1; |
Issue #, if available:
Description of changes:
Adds common steps that allow tracking of the device CPU/RAM use. These will be used by a test that can verify if CPU or RAM use has exceeded a threshold. These CPU/RAM data samples are taken for the general system itself, rather than process specific. This is because it may be difficult to single out a specific component process, but also because we do not know the nature of the tests that may use these steps.
Why is this change necessary:
We will need these steps in order to add a test to the GDK testing template that monitors device CPU/RAM use before and after a component is installed.
How was this change tested:
Steps work with debugging logs when used in a feature file.
Any additional information or context required to review the change:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.