-
Notifications
You must be signed in to change notification settings - Fork 253
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
build: add python 3.11 and 3.12 ci checks #4153
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||||||||||
|
||||||||||||
|
||||||||||||
import json | ||||||||||||
import sys | ||||||||||||
|
||||||||||||
import ddt | ||||||||||||
import mock | ||||||||||||
|
@@ -177,7 +178,10 @@ def test_user_details_uses_jwt(self): | |||||||||||
|
||||||||||||
# Verify the headers passed to the API were correct. | ||||||||||||
expected = {'Authorization': 'JWT {}'.format(token), } | ||||||||||||
self.assertDictContainsSubset(expected, last_request.headers) | ||||||||||||
if sys.version_info > (3, 9): | ||||||||||||
self.assertLessEqual(expected.items(), last_request.headers.items()) | ||||||||||||
else: | ||||||||||||
self.assertDictContainsSubset(expected, last_request.headers) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
||||||||||||
def test_no_user_details(self): | ||||||||||||
""" Verify False is returned when there is a connection error. """ | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ | |||||
Tests for the checkout page. | ||||||
""" | ||||||
|
||||||
|
||||||
import sys | ||||||
from datetime import timedelta | ||||||
|
||||||
import ddt | ||||||
|
@@ -130,7 +130,21 @@ def _assert_success_checkout_page(self, sku=None): | |||||
|
||||||
response = self.client.get(self.path) | ||||||
self.assertEqual(response.status_code, 200) | ||||||
self.assertDictContainsSubset({'course': self.course}, response.context) | ||||||
if sys.version_info > (3, 9): | ||||||
# assertDictContainsSubset is depreciated in python version>3.9 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
??? |
||||||
# context.response return ContextList object, belwo statements will convert it to dict | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
?? |
||||||
# assertLessEqual method is used instead of depreciated assertDictContainsSubset method | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
context = {} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A comment here to explain why this is needed would be helpful. Plus, see if this can be improved. There are a bit too many if-elses in the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. adding a comment for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will also look for the alternative for the if-else. |
||||||
for i, ctx in enumerate(response.context): | ||||||
if isinstance(ctx, dict): | ||||||
context.update(ctx) | ||||||
elif hasattr(ctx, '__iter__') and not isinstance(ctx, str): | ||||||
for item in ctx: | ||||||
if isinstance(item, dict): | ||||||
context.update(item) | ||||||
self.assertLessEqual({'course': self.course}.items(), context.items()) | ||||||
else: | ||||||
self.assertDictContainsSubset({'course': self.course}, response.context) | ||||||
|
||||||
self.assertContains( | ||||||
response, | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
|
||
|
||
import sys | ||
import uuid | ||
|
||
import ddt | ||
|
@@ -122,7 +123,10 @@ def test_post_enterprise_customer_user(self, mock_helpers, expected_return): | |
self.learner.username | ||
) | ||
|
||
self.assertDictContainsSubset(expected_return, response) | ||
if sys.version_info > (3, 9): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment as above |
||
self.assertLessEqual(expected_return.items(), response.items()) | ||
else: | ||
self.assertDictContainsSubset(expected_return, response) | ||
|
||
@responses.activate | ||
def test_ecu_needs_consent(self): | ||
|
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.
I think you can remove this from all the files