-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Mock Scanner, instead of scan the computer running the test. #3173
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
Conversation
|
Ping for review @kiwiflyer @nathanejohnson @DaanHoogland and others. |
|
Still getting failures on mac: |
This allows non linux machines to run the tests without scanning for a non existing /proc/meminfo.
b4dcd9a to
b83a172
Compare
|
Updated the code with a setup method and removed a variable that I left without mocking. Thanks for testing @nathanejohnson. Could you please test it again to make sure that it has been fixed? |
|
still getting failures, but in a new place. I've attached logs |
|
@blueorangutan package |
|
@GabrielBrascher a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2609 |
|
@blueorangutan test |
|
@GabrielBrascher a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@nathanejohnson that is good. At least this PR fixed the test failures from |
wido
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM based on code
|
MemStatTest passes @GabrielBrascher . The following remains: |
|
Trillian test result (tid-3404)
|
borisstoyanov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dhlaluku
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Is this good to merge @GabrielBrascher @wido ? |
|
@rhtyd I submitted some additions to @GabrielBrascher . I hope these can be included in this PR: CLDIN#3 |
* powermock costructor for libvirt wrappers * run test only on linux, don't assume we're on linux * import
|
@rhtyd @DaanHoogland I updated the branch with the changes proposed by Daan. |
|
@nathanejohnson can you give this a test, please? #worksforme |
|
travis test run 7 always seems to fail, lately :( |
|
@DaanHoogland yes, test 7 has been failing constantly, not only in this PR. #3267 looks like a fix for the Travis-CI timeout. |
|
@nathanejohnson are you ok with this PR? |
|
@GabrielBrascher sorry for the late response, builds on my machine now! LGTM! |
This allows computers with Windows or MacOS to run the tests without scanning for
/proc/meminfopath.At the class MemStat.java, the
refresh()method (called in MemStat constructor) has the following lines:When executing
Scanner scanner = new Scanner(f,"UTF-8")it scans the file MEMINFO_FILE (/proc/meminfo); however, if the OS running this test is not a Linux then it throwsFileNotFoundException, failing the test.Thus, by Mocking the Scanner constructor, the
new Scanner(f,"UTF-8")execution returns the desired Scanner.Description
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?