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

Really move verify tests to Python 3 #9571

Merged
merged 5 commits into from
Jul 5, 2018

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Jul 3, 2018

@martinpitt
Copy link
Member Author

@mvollmer: For debugging your tests, either cherry-pick "test: Fix checkSuccess() for Python 3" here, or simply run the test through python2 for the time being (python2 test/verify/check-foo ...).

@mvollmer
Copy link
Member

mvollmer commented Jul 4, 2018

Great work!

@martinpitt martinpitt changed the title WIP: Really move verify tests to Python 3 Really move verify tests to Python 3 Jul 4, 2018
@martinpitt martinpitt added the blocked Don't land until something else happens first (see task list) label Jul 4, 2018
@martinpitt martinpitt removed needswork blocked Don't land until something else happens first (see task list) labels Jul 4, 2018
`b64encode()` returns bytes, convert it to a string for inserting it
into a string.
Python 3's TestCase.run() already does this by itself.
Python 3's unittest calls `tearDown()` before `TestCase.result` gets
updated. So look at `_outcome` instead. This does not exist in Python 2,
so make it version dependent.
In Python3, this exception appears as `testlib.Error:` in tracebacks,
while it is just `Error:` in Python 2. Normalize it so that our existing
naughty overrides continue to work.
Commit 6939ef2 missed run-tests, so while running individual check-*
tests was using Python 3, running the entire suite was not.

Closes cockpit-project#9571
@martinpitt
Copy link
Member Author

Only one well-known flake, I think this is green enough. (We can't run tests ATM)

@@ -255,13 +255,13 @@ class TestConnection(MachineCase):

# login handler: wrong password
headers = m.execute("curl -s --head --header 'Authorization: Basic {}' http://172.27.0.15:9090/cockpit/login".format(
base64.b64encode(b"admin:hahawrong")))
base64.b64encode(b"admin:hahawrong").decode()))
Copy link
Member

Choose a reason for hiding this comment

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

b64encode returns bytes?! this is some kind of awful :(

(I checked, and indeed, it seems that there is no better way to handle this)

@@ -286,9 +286,10 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group
self.allow_restart_journal_messages()

def curl_auth(self, url, userpass):
header = "Authorization: Basic " + base64.b64encode(userpass)
header = "Authorization: Basic " + base64.b64encode(userpass.encode()).decode()
Copy link
Member

Choose a reason for hiding this comment

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

lol

@@ -221,6 +221,9 @@ def normalize_traceback(trace):
# replace noise in SELinux violations
trace = re.sub('audit\([0-9.:]+\)', 'audit(0)', trace)
trace = re.sub(r'\b(pid|ino)=[0-9]+ ', r'\1=0 ', trace)

# in Python 3, testlib.Error is shown with namespace
trace = re.sub('testlib\.Error', 'Error', trace)
Copy link
Member

Choose a reason for hiding this comment

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

I'd strongly consider normalising this in the other direction (and updating the naughties). It seems that py3 is the expected norm now, so why not treat it that way?

Copy link
Member Author

Choose a reason for hiding this comment

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

I filed issue #9591 about it. I'm still hoping that the actually printed format can be fixed to just Error: again, I just didn't see a good way of doing this. I don't want to change all the naughties until this settled down and we also port the selenium, avocado, and container tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants