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

Clean up tests #58

Merged
merged 14 commits into from
Feb 16, 2018
Merged

Clean up tests #58

merged 14 commits into from
Feb 16, 2018

Conversation

emlun
Copy link
Member

@emlun emlun commented Feb 15, 2018

The integration tests now reliably pass in both Linux and Mac!

@emlun emlun self-assigned this Feb 15, 2018
ykman/device.py Outdated
@@ -180,6 +184,10 @@ def use_transport(self, transport):
assert dev.mode == my_mode
return dev

def close(self):
logger.debug('close')
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this log message or improve it a bit.

ykman/driver.py Outdated
@@ -64,3 +68,6 @@ def read_capabilities(self):
def guess_version(self):
# Second arg is True if the version is certain, False if not.
return (0, 0, 0), False

def close(self):
logger.debug('close')
Copy link
Contributor

Choose a reason for hiding this comment

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

See above (about log msg)

from test.util import ykman_cli


@unittest.skipIf(
os.environ.get('INTEGRATION_TESTS') != 'TRUE', 'INTEGRATION_TESTS != TRUE')
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this? Shouldn't the python only unittests be able to run without the libraries in place?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oooh, now I see why this was there. Good catch, thanks! This should just be updated to use the new environment variable, then.

Copy link
Member Author

@emlun emlun Feb 16, 2018

Choose a reason for hiding this comment

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

Case in point: all CI builds failed... �

@dagheyman
Copy link
Contributor

Very nice, I look forward to approving :)

@emlun emlun merged commit 5df4878 into master Feb 16, 2018
@emlun emlun deleted the cleanup-tests branch February 16, 2018 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants