Conversation
|
Even with this fix, I may have misinterpreted #8449 to mean that Python 2 wasn't relevant for integration testing anymore, but I see it's more nuanced than that. Here's a patch for that, feel free to squash: From 5ace8e59642e64855f2c6258586894b70937f7f1 Mon Sep 17 00:00:00 2001
From: Alex Zorin <alex@zorin.id.au>
Date: Wed, 18 Nov 2020 09:51:42 +1100
Subject: [PATCH] certbot-ci: fix py2 crash in dns_server
---
.../utils/dns_server.py | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/certbot-ci/certbot_integration_tests/utils/dns_server.py b/certbot-ci/certbot_integration_tests/utils/dns_server.py
index 3d2f50d3b..779d736e3 100644
--- a/certbot-ci/certbot_integration_tests/utils/dns_server.py
+++ b/certbot-ci/certbot_integration_tests/utils/dns_server.py
@@ -40,6 +40,8 @@ class DNSServer(object):
self.bind_root = tempfile.mkdtemp()
+ self.process = None
+
self.dns_xdist = {
'address': BIND_BIND_ADDRESS[0],
'port': BIND_BIND_ADDRESS[1]
@@ -60,11 +62,12 @@ class DNSServer(object):
def stop(self):
"""Stop the DNS server, and clean its resources"""
- try:
- self.process.terminate()
- self.process.wait()
- except BaseException as e:
- print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr)
+ if self.process:
+ try:
+ self.process.terminate()
+ self.process.wait()
+ except BaseException as e:
+ print("BIND9 did not stop cleanly: {}".format(e), file=sys.stderr)
shutil.rmtree(self.bind_root, ignore_errors=True)
@@ -74,7 +77,8 @@ class DNSServer(object):
def _configure_bind(self):
"""Configure the BIND9 server based on the prebaked configuration"""
bind_conf_src = resource_filename('certbot_integration_tests', 'assets/bind-config')
- shutil.copytree(bind_conf_src, self.bind_root, dirs_exist_ok=True)
+ for dir in ('conf', 'zones'):
+ shutil.copytree(os.path.join(bind_conf_src, dir), os.path.join(self.bind_root, dir))
def _start_bind(self):
"""Launch the BIND9 server as a Docker container"""
--
2.25.1 |
Ah yeah. To provide more context, we currently run integration tests with every version of Python we claim to support in our nightly/full tests. What that PR did was change the version of Python used when running integration tests on PRs. Thanks for the patch! I have tests running with that change at https://dev.azure.com/certbot/certbot/_build/results?buildId=3032&view=results. |
There was a problem hiding this comment.
Provided that https://dev.azure.com/certbot/certbot/_build/results?buildId=3032&view=results will pass, this LGTM. Sorry for not having run the full test suite to validate the integration tests.
|
No worries. I'm somewhat tempted to rethink our split between tests run on PRs and the full test suite given how often we're running nightly tests manually, but I think it's a little tricky and I don't immediately see a good way to do this. |
It looks like #8448 added some code that only worked on Python 3 which caused our Python 2 integration tests to fail. See https://dev.azure.com/certbot/certbot/_build/results?buildId=3026&view=results (and ignore the
mypyfailure).This PR fixes the problem. You can see test passing with this change at https://dev.azure.com/certbot/certbot/_build/results?buildId=3027&view=results.