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

rewrite test_get_recovery_data #2806

Merged

Conversation

tedder
Copy link
Contributor

@tedder tedder commented Sep 2, 2018

What does this PR do and why is it necessary?

py3 compatability: I was pulling my hair out with some py3 compatability, so .. I ended up basically rewriting this test. Basically a big refactor, still compatible with py2.

How was it tested? How can it be tested by the reviewer?

with nose

Any background context you want to provide?

What are the relevant tickets if any?

Screenshots (if appropriate)

Further notes

I was pulling my hair out with some py3 compatability, so .. I ended up basically rewriting it. Basically a big refactor, still compatible with py2.
tedder added a commit to tedder/OctoPrint that referenced this pull request Sep 2, 2018
Some prepwork for py3. Tossing these together:
- assertItemsEqual doesn't do what it says on the box. It only said "hey, these lists have the same number of items". Not content or anything. Changing that to SetEqual, which is much closer to what is wanted- ensuring that lists have the same content without caring about their order.
- use assertRaises instead of try/catch
- update some mocks, especially mock_open.
- include the refactored test_get_recovery_data sent to the maintenance branch in OctoPrint#2806.
@foosel foosel added the needs review This PR needs a review label Sep 5, 2018
@foosel foosel self-assigned this Sep 5, 2018
@foosel foosel merged commit a920c4f into OctoPrint:maintenance Sep 18, 2018
@tedder tedder deleted the 201809_rewrite_test_get_recovery branch September 18, 2018 15:34
@foosel foosel modified the milestone: 1.3.10 Oct 31, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs review This PR needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants