Merge branch 'juerg/synthetic-file-index' into 'master'

node.pyx: _SYNTHETIC_FILE_INDEX must not be module-private

See merge request BuildStream/buildstream!1576

4 files changed, 33 insertions, 16 deletions

diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx
index 73911634c..b39d01db6 100644
--- a/src/buildstream/node.pyx
+++ b/src/buildstream/node.pyx
@@ -125,10 +125,10 @@ cdef class Node:
         """
         if value:
             return __new_node_from_dict(value, MappingNode.__new__(
-                MappingNode, __SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), {}))
+                MappingNode, _SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), {}))
         else:
             # We got an empty dict, we can shortcut
-            return MappingNode.__new__(MappingNode, __SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), {})
+            return MappingNode.__new__(MappingNode, _SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), {})
 
     cpdef ProvenanceInformation get_provenance(self):
         """A convenience accessor to obtain the node's :class:`.ProvenanceInformation`
@@ -1109,7 +1109,7 @@ cdef class MappingNode(Node):
 
         # Clobber the provenance of the target mapping node if we're not
         # synthetic.
-        if self.file_index != __SYNTHETIC_FILE_INDEX:
+        if self.file_index != _SYNTHETIC_FILE_INDEX:
             target.file_index = self.file_index
             target.line = self.line
             target.column = self.column
@@ -1140,7 +1140,7 @@ cdef class MappingNode(Node):
                 value = None
             else:
                 value = default_constructor.__new__(
-                    default_constructor, __SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), default)
+                    default_constructor, _SYNTHETIC_FILE_INDEX, 0, __next_synthetic_counter(), default)
 
         return value
 
@@ -1437,7 +1437,7 @@ cdef class ProvenanceInformation:
         cdef __FileInfo fileinfo
 
         self._node = nodeish
-        if (nodeish is None) or (nodeish.file_index == __SYNTHETIC_FILE_INDEX):
+        if (nodeish is None) or (nodeish.file_index == _SYNTHETIC_FILE_INDEX):
             self._filename = ""
             self._shortname = ""
             self._displayname = ""
@@ -1469,6 +1469,13 @@ cdef class ProvenanceInformation:
 #                BuildStream Private methods                #
 #############################################################
 
+# Purely synthetic nodes will have _SYNTHETIC_FILE_INDEX for the file number, have line number
+# zero, and a negative column number which comes from inverting the next value
+# out of this counter.  Synthetic nodes created with a reference node will
+# have a file number from the reference node, some unknown line number, and
+# a negative column number from this counter.
+cdef int _SYNTHETIC_FILE_INDEX = -1
+
 # _assert_symbol_name()
 #
 # A helper function to check if a loaded string is a valid symbol
@@ -1546,7 +1553,7 @@ cdef Py_ssize_t _create_new_file(str filename, str shortname, str displayname, o
 cdef void _set_root_node_for_file(Py_ssize_t file_index, MappingNode contents) except *:
     cdef __FileInfo f_info
 
-    if file_index != __SYNTHETIC_FILE_INDEX:
+    if file_index != _SYNTHETIC_FILE_INDEX:
         f_info = <__FileInfo> __FILE_LIST[file_index]
         f_info.toplevel = contents
 
@@ -1577,17 +1584,22 @@ def _new_synthetic_file(str filename, object project=None):
     return node
 
 
+# _reset_global_state()
+#
+# This resets the global variables __FILE_LIST and __counter to their initial
+# state. This is used by the test suite to improve isolation between tests
+# running in the same process.
+#
+def _reset_global_state():
+    global __FILE_LIST, __counter
+    __FILE_LIST = []
+    __counter = 0
+
+
 #############################################################
 #                 Module local helper Methods               #
 #############################################################
 
-# Purely synthetic nodes will have _SYNTHETIC_FILE_INDEX for the file number, have line number
-# zero, and a negative column number which comes from inverting the next value
-# out of this counter.  Synthetic nodes created with a reference node will
-# have a file number from the reference node, some unknown line number, and
-# a negative column number from this counter.
-cdef int __SYNTHETIC_FILE_INDEX = -1
-
 # File name handling
 cdef list __FILE_LIST = []
 
diff --git a/src/buildstream/testing/_fixtures.py b/src/buildstream/testing/_fixtures.py
index 862cebe87..2684782a1 100644
--- a/src/buildstream/testing/_fixtures.py
+++ b/src/buildstream/testing/_fixtures.py
@@ -17,7 +17,7 @@
 import psutil
 import pytest
 
-from buildstream import utils
+from buildstream import node, utils
 
 # Catch tests that don't shut down background threads, which could then lead
 # to other tests hanging when BuildStream uses fork().
@@ -29,3 +29,8 @@ def thread_check():
 
     yield
     assert utils._is_single_threaded()
+
+# Reset global state in node.pyx to improve test isolation
+@pytest.fixture(autouse=True)
+def reset_global_node_state():
+    node._reset_global_state()
diff --git a/src/buildstream/testing/_sourcetests/conftest.py b/src/buildstream/testing/_sourcetests/conftest.py
index f16c1e9ad..64dd404ef 100644
--- a/src/buildstream/testing/_sourcetests/conftest.py
+++ b/src/buildstream/testing/_sourcetests/conftest.py
@@ -14,4 +14,4 @@
 #  You should have received a copy of the GNU Lesser General Public
 #  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 
-from .._fixtures import thread_check  # pylint: disable=unused-import
+from .._fixtures import reset_global_node_state, thread_check  # pylint: disable=unused-import
diff --git a/tests/conftest.py b/tests/conftest.py
index 0e39943d3..9189d263e 100755
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -23,7 +23,7 @@ import os
 import pytest
 
 from buildstream.testing import register_repo_kind, sourcetests_collection_hook
-from buildstream.testing._fixtures import thread_check  # pylint: disable=unused-import
+from buildstream.testing._fixtures import reset_global_node_state, thread_check  # pylint: disable=unused-import
 from buildstream.testing._forked import forked_run_report
 from buildstream.testing.integration import integration_cache  # pylint: disable=unused-import