From bba63db8be9119dd82520c477bf087e220548add Mon Sep 17 00:00:00 2001
From: Shahzeb Siddiqui <shahzebmsiddiqui@gmail.com>
Date: Thu, 8 Dec 2022 16:39:30 -0500
Subject: [PATCH] fix bug in 'buildtest bc edit' where test were not openning
 in editor

---
 buildtest/cli/buildspec.py  | 13 +++++++------
 tests/cli/test_buildspec.py |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/buildtest/cli/buildspec.py b/buildtest/cli/buildspec.py
index ced2dda3a..5e84d9ff3 100644
--- a/buildtest/cli/buildspec.py
+++ b/buildtest/cli/buildspec.py
@@ -986,7 +986,7 @@ def print_paths(self):
             console.print(path)
 
 
-def edit_buildspec_test(test_names, configuration, editor, test=None):
+def edit_buildspec_test(test_names, configuration, editor):
     """Open a list of test names in editor mode defined by ``EDITOR`` environment otherwise resort to ``vim``.
     This method will search for buildspec cache and find path to buildspec file corresponding to test name and open
     file in editor. If multiple test are specified via ``buildtest buildspec edit-test`` then each file will be open and
@@ -996,7 +996,6 @@ def edit_buildspec_test(test_names, configuration, editor, test=None):
         test_names (list): A list of test names to open in editor
         configuration (buildtest.config.SiteConfiguration): An instance of SiteConfiguration class
         editor (str): Path to editor to use when opening file
-        test (bool): A boolean to determine whether to open file for editing, by default `test=None` indicates file will be open in editor. If its True this method will return immediately which is useful when running regression test.
     """
     cache = BuildspecCache(configuration=configuration)
 
@@ -1006,7 +1005,9 @@ def edit_buildspec_test(test_names, configuration, editor, test=None):
             continue
 
         buildspec = cache.lookup_buildspec_by_name(name)
-        if test is None:
+
+        # only used for regression testing to ensure test is not stuck for closing file
+        if not editor:
             editor = "echo"  # Doesnt call the editor.
 
         subprocess.call([editor, buildspec])
@@ -1021,14 +1022,13 @@ def edit_buildspec_test(test_names, configuration, editor, test=None):
         console.print(f"[green]{buildspec} is valid")
 
 
-def edit_buildspec_file(buildspecs, configuration, editor, test=None):
+def edit_buildspec_file(buildspecs, configuration, editor):
     """Open buildspec in editor and validate buildspec with parser. This method is invoked by command ``buildtest buildspec edit-file``.
 
     Args:
         buildspec (str): Path to buildspec file to edit
         configuration (buildtest.config.SiteConfiguration): An instance of SiteConfiguration class
         editor (str): Path to editor to use when opening file
-        test (bool): A boolean to determine whether to open file for editing, by default `test=None` indicates file will be open in editor. If its True this method will return immediately which is useful when running regression test.
     """
     for file in buildspecs:
 
@@ -1038,7 +1038,8 @@ def edit_buildspec_file(buildspecs, configuration, editor, test=None):
                 f"buildspec: {buildspec} is a directory, please specify a file type"
             )
             continue
-        if test is None:
+        # only used for regression testing to ensure test is not stuck for closing file
+        if not editor:
             editor = "cat"
         subprocess.call([editor, buildspec])
 
diff --git a/tests/cli/test_buildspec.py b/tests/cli/test_buildspec.py
index fe4a04100..e140c5fd2 100644
--- a/tests/cli/test_buildspec.py
+++ b/tests/cli/test_buildspec.py
@@ -223,7 +223,7 @@ def test_edit_file():
     edit_buildspec_file(
         buildspecs=[os.path.join(BUILDTEST_ROOT, "tutorials", "vars.yml")],
         configuration=configuration,
-        editor="",
+        editor=None,
     )