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

Add spm tests to Windows suite #49875

Merged
merged 3 commits into from
Oct 5, 2018
Merged

Add spm tests to Windows suite #49875

merged 3 commits into from
Oct 5, 2018

Conversation

dwoz
Copy link
Contributor

@dwoz dwoz commented Oct 3, 2018

What does this PR do?

  • Adds integration.spm.* tests to whitelist.txt for Windows.
  • Close sqlite connections so test tearDown methods can remove temporary directories

Tests written?

No

Commits signed with GPG?

Yes

@ghost ghost requested review from a team October 3, 2018 08:28
@dwoz dwoz changed the title Add spm tests for Windows Add spm tests to Windows suite Oct 3, 2018
@gtmanfred
Copy link
Contributor

nice! did you get spm to work on windows? #43174

@rallytime
Copy link
Contributor

A couple of these new test additions are failing: https://jenkinsci.saltstack.com/job/pr-kitchen-windows2016-py3/job/PR-49875/1/

@rallytime
Copy link
Contributor

@dwoz Looks like there is an spm test failing on the windows test. Can you take another look?

if conn is None:
close = True
Copy link
Contributor

Choose a reason for hiding this comment

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

You actually can just None the conn on .close() instead of having close = False | True:

if conn is not None:
    conn.close()
    conn = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, We do not want to close a connection passed in via the conn keyword argument.

@dwoz dwoz merged commit eee82d3 into saltstack:2018.3 Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants