Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

Refactor Linux memory measurement tests #100

Merged
merged 2 commits into from
Mar 2, 2020

Conversation

dabonnie
Copy link
Contributor

Closes #96

This refactor uses futures in order to wait for asynchronous events, instead of making strict time assumptions. Note that the unit test no longer tests for multiple received messages, but this PR adds this check to e2e testing: #98

Marking as draft as test documentation is currently needed. Note that this test was run for >7000 times without failure using
./test_linux_memory_measurement_node --gtest_repeat=-1 --gtest_break_on_failure.

Signed-off-by: Devin Bonnie dbbonnie@amazon.com

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
"MemTotal: 16304208 kB\n"
"MemFree: 827568 kB\n"
"MemAvailable: 4838060 kB\n",
ExpectedStatistics StatisticDataToExpectedStatistics(const StatisticData & src)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be moved to test_utilities

@dabonnie
Copy link
Contributor Author

The continuous gtests exited with this failure:

Calculated port number is too high. Probably the domainId is over 232, there are too much participants created or portBase is too high.                            
2020-02-28 14:44:56.043 [RTPS Error] Calculated port number is too high. Probably the domainId is over 232, there are too much participants created or portBase is too high. -> Function g
etUnicastPort 

@codecov
Copy link

codecov bot commented Feb 29, 2020

Codecov Report

Merging #100 into master will increase coverage by 0.02%.
The diff coverage is 38.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   39.33%   39.36%   +0.02%     
==========================================
  Files          36       36              
  Lines        1449     1481      +32     
  Branches      869      889      +20     
==========================================
+ Hits          570      583      +13     
- Misses         66       69       +3     
- Partials      813      829      +16
Flag Coverage Δ
#unittests 39.36% <38.18%> (+0.02%) ⬆️
Impacted Files Coverage Δ
...etrics_collector/test_linux_memory_measurement.cpp 30.99% <38.18%> (+2.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b22930...3ecb2b6. Read the comment docs.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie dabonnie marked this pull request as ready for review March 2, 2020 20:29
Copy link
Contributor

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

This seems good

ASSERT_EQ(State::PRIMARY_STATE_ACTIVE, test_measure_linux_memory_->get_current_state().id());
ASSERT_TRUE(test_measure_linux_memory_->IsStarted());

//
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra comment line?

Copy link
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

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

Nice work with the PromiseSetter approach.

@dabonnie dabonnie merged commit 784b8c0 into master Mar 2, 2020
@dabonnie dabonnie deleted the dabonnie/fix-linux-memory-measurement-test branch March 2, 2020 23:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LinuxMemoryMeasurementTestFixture.TestPublishMetricsMessage test is flaky
3 participants