From 5f3c0f3cb2f9eb71fde5dd3a86eac6ed71149541 Mon Sep 17 00:00:00 2001
From: Clark Boylan <clark.boylan@gmail.com>
Date: Tue, 10 Dec 2019 14:42:50 -0800
Subject: [PATCH] Fix python3 compat in tox siblings handling

This fixes a byte vs str problem on python3 in the tox role's sibling
library.

As part of this fix add unit testing of the function that was failing
under python3 and functional testing of the role that covers siblings.

Change-Id: I2df4ddce91dfd53f5c4177a19288e97120bc3e3b
---
 roles/tox/library/test-constraints.txt        |  2 ++
 .../test_tox_install_sibling_packages.py      | 26 +++++++++++++++++++
 .../library/tox_install_sibling_packages.py   |  3 ++-
 zuul-tests.d/python-jobs.yaml                 | 15 +++++++++++
 zuul-tests.d/test-constraints.txt             |  3 +++
 5 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 roles/tox/library/test-constraints.txt
 create mode 100644 zuul-tests.d/test-constraints.txt

diff --git a/roles/tox/library/test-constraints.txt b/roles/tox/library/test-constraints.txt
new file mode 100644
index 000000000..d2c6e43fe
--- /dev/null
+++ b/roles/tox/library/test-constraints.txt
@@ -0,0 +1,2 @@
+zuul===3.12.0
+doesnotexistonpypi===0.0.1
diff --git a/roles/tox/library/test_tox_install_sibling_packages.py b/roles/tox/library/test_tox_install_sibling_packages.py
index 6b145c28a..8ab245c8f 100644
--- a/roles/tox/library/test_tox_install_sibling_packages.py
+++ b/roles/tox/library/test_tox_install_sibling_packages.py
@@ -18,10 +18,12 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import os
 import sys
 import testtools
 
 from .tox_install_sibling_packages import get_installed_packages
+from .tox_install_sibling_packages import write_new_constraints_file
 
 
 class TestToxInstallSiblingPackages(testtools.TestCase):
@@ -35,3 +37,27 @@ class TestToxInstallSiblingPackages(testtools.TestCase):
         #               but this might fail later if we stop adding Zuul
         #               in the unit tests.
         self.assertIn("zuul", pkgs)
+
+    def test_write_new_constraints_file(self):
+        # NOTE(mnaser): Given that we run our tests inside Tox, we can
+        #               leverage the tox virtual environment we use in
+        #               unit tests instead of mocking up everything.
+        pkgs = get_installed_packages(sys.executable)
+
+        # NOTE(mnaser): Zuul should be installed in this virtualenv
+        #               but this might fail later if we stop adding Zuul
+        #               in the unit tests.
+        test_constraints = os.path.join(os.path.dirname(__file__),
+                                        'test-constraints.txt')
+        constraints = write_new_constraints_file(test_constraints, pkgs)
+
+        def cleanup_constraints_file():
+            if os.path.exists(constraints):
+                os.unlink(constraints)
+        self.addCleanup(cleanup_constraints_file)
+
+        self.assertTrue(os.path.exists(constraints))
+        with open(constraints) as f:
+            s = f.read()
+            self.assertNotIn("zuul", s)
+            self.assertIn("doesnotexistonpypi", s)
diff --git a/roles/tox/library/tox_install_sibling_packages.py b/roles/tox/library/tox_install_sibling_packages.py
index abec207e6..915186c1b 100644
--- a/roles/tox/library/tox_install_sibling_packages.py
+++ b/roles/tox/library/tox_install_sibling_packages.py
@@ -121,7 +121,8 @@ def get_installed_packages(tox_python):
 
 
 def write_new_constraints_file(constraints, packages):
-    with tempfile.NamedTemporaryFile(delete=False) as constraints_file:
+    with tempfile.NamedTemporaryFile(mode='w', delete=False) \
+            as constraints_file:
         constraints_lines = open(constraints, 'r').read().split('\n')
         for line in constraints_lines:
             package_name = line.split('===')[0]
diff --git a/zuul-tests.d/python-jobs.yaml b/zuul-tests.d/python-jobs.yaml
index 4b9485532..8ae2362ef 100644
--- a/zuul-tests.d/python-jobs.yaml
+++ b/zuul-tests.d/python-jobs.yaml
@@ -7,6 +7,20 @@
     vars:
       role_name: ensure-sphinx
 
+- job:
+    name: zuul-jobs-test-tox-siblings
+    description: Test the tox role's sibling functionality
+    files:
+      - roles/tox/.*
+    run: test-playbooks/simple-role-test.yaml
+    vars:
+      role_name: tox
+      tox_envlist: docs
+      tox_constraints_file: '{{ zuul.project.src_dir }}/zuul-tests.d/test-constraints.txt'
+    required-projects:
+      - zuul/zuul
+      - zuul/nodepool
+
 # -* AUTOGENERATED *-
 #  The following project section is autogenerated by
 #    tools/update-test-platforms.py
@@ -16,5 +30,6 @@
     check:
       jobs: &id001
         - zuul-jobs-test-ensure-sphinx
+        - zuul-jobs-test-tox-siblings
     gate:
       jobs: *id001
diff --git a/zuul-tests.d/test-constraints.txt b/zuul-tests.d/test-constraints.txt
new file mode 100644
index 000000000..8b0f3353f
--- /dev/null
+++ b/zuul-tests.d/test-constraints.txt
@@ -0,0 +1,3 @@
+nodepool===3.10.0
+zuul===3.12.0
+openstacksdk==0.39.0