From 64a379aeb2b456a634b1cbfe3dd1b4d1c5d54588 Mon Sep 17 00:00:00 2001
From: Bob Swift <bswift@rsds.ca>
Date: Mon, 2 Mar 2020 14:45:22 -0700
Subject: [PATCH] Use thread-safe function stack.  Add platform-specific tests.

---
 picard/script.py    |  9 +++++----
 test/test_script.py | 38 ++++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/picard/script.py b/picard/script.py
index 3fec86a85..d0842bf62 100644
--- a/picard/script.py
+++ b/picard/script.py
@@ -41,6 +41,7 @@ import datetime
 from functools import reduce
 from inspect import getfullargspec
 import operator
+from queue import LifoQueue
 import re
 import unicodedata
 
@@ -150,10 +151,10 @@ class ScriptFunction(object):
             args = [arg.eval(parser) for arg in self.args]
         else:
             args = self.args
-        parser._function_stack.append(self.name)
+        parser._function_stack.put(self.name)
         # Save return value to allow removing function from the stack on successful completion
         return_value = function(parser, *args)
-        parser._function_stack = parser._function_stack[:-1]
+        parser._function_stack.get()
         return return_value
 
 
@@ -185,7 +186,7 @@ Grammar:
     _cache = {}
 
     def __init__(self):
-        self._function_stack = []
+        self._function_stack = LifoQueue()
 
     def __raise_eof(self):
         raise ScriptEndOfFile("Unexpected end of script at position %d, line %d" % (self._x, self._y))
@@ -1281,7 +1282,7 @@ def func_datetime(parser, format=None):
     except ValueError:
         raise ScriptRuntimeError(
             "Unsupported format code",
-            parser._function_stack[-1],
+            parser._function_stack.get(),
             parser._x,
             parser._y
         )
diff --git a/test/test_script.py b/test/test_script.py
index 76cfad98f..32f216239 100644
--- a/test/test_script.py
+++ b/test/test_script.py
@@ -31,7 +31,6 @@
 
 import copy
 import datetime
-from unittest import skipUnless
 from unittest.mock import MagicMock
 
 from test.picardtestcase import PicardTestCase
@@ -39,7 +38,6 @@ from test.picardtestcase import PicardTestCase
 from picard import config
 from picard.cluster import Cluster
 from picard.const import DEFAULT_FILE_NAMING_FORMAT
-from picard.const.sys import IS_WIN
 from picard.metadata import Metadata
 from picard.script import (
     MULTI_VALUED_JOINER,
@@ -1150,7 +1148,7 @@ class ScriptParserTest(PicardTestCase):
             self.parser.eval("$slice(abc; def),0,1,:,extra")
 
     def test_cmd_datetime(self):
-        # Save origninal datetime object and substitute one returning
+        # Save original datetime object and substitute one returning
         # a fixed now() value for testing.
         original_datetime = datetime.datetime
         datetime.datetime = _DateTime
@@ -1179,24 +1177,36 @@ class ScriptParserTest(PicardTestCase):
             # Restore original datetime object
             datetime.datetime = original_datetime
 
-    @skipUnless(IS_WIN, "windows test")
-    def test_cmd_datetime_windows(self):
-        ''' Test failure specific to Windows only. '''
-        # Save origninal datetime object and substitute one returning
+    def test_cmd_datetime_platform_dependent(self):
+        # Platform dependent testing because different platforms (both os and Python version)
+        # support some format arguments differently.
+        possible_tests = (
+            '%',    # Hanging % at end of format
+            '%-d',  # Non zero-padded day
+            '%-m',  # Non zero-padded month
+            '%3Y',  # Length specifier shorter than string
+        )
+        tests_to_run = []
+        # Get list of tests for unsupported format codes
+        for test_case in possible_tests:
+            try:
+                datetime.datetime.now().strftime(test_case)
+            except ValueError:
+                tests_to_run.append(test_case)
+        if not tests_to_run:
+            self.skipTest('datetime module supports all test cases')
+        # Save original datetime object and substitute one returning
         # a fixed now() value for testing.
         original_datetime = datetime.datetime
         datetime.datetime = _DateTime
 
         try:
             context = Metadata()
-            # Tests with Windows-specific format
-            self.assertScriptResultEquals(r"$datetime(\%Y-\%#m-\%#d)", "2020-1-2", context)
-            # Tests with invalid format codes (Windows does not support -d or -m codes)
             areg = r"^\$datetime:\d+:\d+: Unsupported format code"
-            with self.assertRaisesRegex(ScriptRuntimeError, areg):
-                self.parser.eval(r"$datetime(\%-d)")
-            with self.assertRaisesRegex(ScriptRuntimeError, areg):
-                self.parser.eval(r"$datetime(\%-m)")
+            # Tests with invalid format code (platform dependent tests)
+            for test_case in tests_to_run:
+                with self.assertRaisesRegex(ScriptRuntimeError, areg):
+                    self.parser.eval(r'$datetime(\{0})'.format(test_case))
         except (AssertionError, ValueError, IndexError) as err:
             raise err
         finally: