Updates to Drop Python 3.6 #195
Conversation
jvanderaa
requested review from
chadell,
glennmatthews,
pke11y and
scetron
as code owners
|
|
| @classmethod | ||
| def setUp(cls): | ||
| def setUp(cls): # pylint: disable=arguments-differ |
There was a problem hiding this comment.
This sure looks wrong to me - should either be @classmethod setUpTestData(cls) or just simply setUp(self), right?
There was a problem hiding this comment.
@jvanderaa have you taken care of this?
There was a problem hiding this comment.
What is the thought? I am not clear in understanding what either is doing immediately. Just disabling pylint rule from the pylint updates.
There was a problem hiding this comment.
If not I will take a look more upon my return.
There was a problem hiding this comment.
@classmethod setUpTestData(cls) runs once before all tests in this TestCase, setUp(self) runs before each individual test in this TestCase. @classmethod setUp(cls) is some sort of weird hybrid between these two and is incorrect. I suspect that changing it to setUp(self) should be the correct fix here.
There was a problem hiding this comment.
Thanks @jvanderaa. Are the 'test_has_advanced_tab' now required or are these just place holders?
|
The tests are auto generated as part of the inheritance for the test case. We need to adjust the HTML templates to have a new base to get these views for free. @scetron |
| @@ -132,7 +132,10 @@ jobs: | |||
| fail-fast: true | |||
| matrix: | |||
| python-version: ["3.7", "3.8", "3.9"] | |||
| nautobot-version: ["1.1.5", "1.2.11", "1.3.7"] | |||
| nautobot-version: ["1.3.8"] | |||
There was a problem hiding this comment.
which was the problem with mysql?
BTW, would make sense to leave the nautboto-version to "stable" to always check against hte latest by default? (don't recall if we already commented this point)
There was a problem hiding this comment.
stable doesn't work. There is a parser error.
There was a problem hiding this comment.
I'm just trying to get things into a working state. What didn't work with MySQL is that I didn't have a matrix set up for it.
There was a problem hiding this comment.
sorry Josh, I don't get it, because the matrix was already set up before, no?
There was a problem hiding this comment.
Not for stable. It was set up to test individual versions.
There was a problem hiding this comment.
yes, I understand, not for stable, but the matrix was working well for mysql before, no?
There was a problem hiding this comment.
Yes it was. I guess the wording of fix mysql was poor. Probably should have just been update mysql matrix.
There was a problem hiding this comment.
to not block you, let's move this forward, but I would support adding the workaround for the stable that @glennmatthews suggested (in another PR if you want)
jvanderaa
dismissed stale reviews from glennmatthews, scetron, and chadell
via
9d2b62a
Looking to drop the Python3.6 compatibility.