Complete tests for context and fix discovered bugs

Namely the impliit use without checking of current_context
which can cause segfault if it is None.

src/palace.pyx

@@ -276,16 +276,17 @@ def use_context(context: Optional[Context],
                 thread: Optional[bool] = None) -> None:
     """Make the specified context current for OpenAL operations.
 
-    If `thread` is set to `True`, make the context current
+    This fails silently if the given context has been destroyed.
+    In case `thread` is not specified, fallback to preference made by
+    `thread_local`.
+
+    If `thread` is `True`, make the context current
     for OpenAL operations on the calling thread only.
     This requires the non-device-specific as well as the context's
     device `ALC_EXT_thread_local_context` extension to be available.
-
-    In case `thread` is not specified, fallback to preference made by
-    `thread_local`.
     """
     cdef alure.Context alure_context = <alure.Context> nullptr
-    if context is not None: alure_context = (<Context> context).impl
+    if context: alure_context = (<Context> context).impl
     if thread is None: thread = _thread
     if thread:
         alure.Context.make_thread_current(alure_context)
@@ -322,6 +323,7 @@ def cache(names: Iterable[str], context: Optional[Context] = None) -> None:
     cdef vector[alure.StringView] alure_names
     for name in std_names: alure_names.push_back(<alure.StringView> name)
     if context is None: context = current_context()
+    if not context: raise RuntimeError('there is no context current')
     (<Context> context).impl.precache_buffers_async(alure_names)
 
 
@@ -336,6 +338,7 @@ def free(names: Iterable[str], context: Optional[Context] = None) -> None:
         If there is neither any context specified nor current.
     """
     if context is None: context = current_context()
+    if not context: raise RuntimeError('there is no context current')
     cdef alure.Context alure_context = (<Context> context).impl
     # Cython cannot infer collection types yet.
     cdef vector[string] std_names = list(names)
@@ -368,6 +371,7 @@ def decode(name: str, context: Optional[Context] = None) -> Decoder:
             return find_resource(subst(name), subst)
 
     if context is None: context = current_context()
+    if not context: raise RuntimeError('there is no context current')
     resource = find_resource(
         name, context.message_handler.resource_not_found)
     for decoder_factory in decoder_factories:
@@ -788,11 +792,7 @@ cdef class Context:
             alure_sample_type = SAMPLE_TYPES.at(sample_type)
         except IndexError:
             raise ValueError(f'invalid sample type: {sample_type}') from None
-        try:
+        return self.impl.is_supported(alure_channel_config, alure_sample_type)
-            return self.impl.is_supported(alure_channel_config,
-                                          alure_sample_type)
-        except IndexError as e:
-            raise ValueError(str(e)) from None
 
     @getter
     def available_resamplers(self) -> List[str]:
@@ -982,6 +982,7 @@ cdef class Buffer:
 
     def __init__(self, name: str, context: Optional[Context] = None) -> None:
         if context is None: context = current_context()
+        if not context: raise RuntimeError('there is no context current')
         self.context, self.name = context, name
         self.impl = self.context.impl.find_buffer(self.name)
         if not self:
@@ -1181,6 +1182,7 @@ cdef class Source:
 
     def __init__(self, context: Optional[Context] = None) -> None:
         if context is None: context = current_context()
+        if not context: raise RuntimeError('there is no context current')
         self.impl = (<Context> context).impl.create_source()
 
     def __enter__(self) -> Source: return self
@@ -1817,6 +1819,7 @@ cdef class SourceGroup:
 
     def __init__(self, context: Optional[Context] = None) -> None:
         if context is None: context = current_context()
+        if not context: raise RuntimeError('there is no context current')
         self.impl = (<Context> context).impl.create_source_group()
 
     def __enter__(self) -> SourceGroup: return self
@@ -1969,6 +1972,7 @@ cdef class BaseEffect:
 
     def __init__(self, context: Optional[Context] = None) -> None:
         if context is None: context = current_context()
+        if not context: raise RuntimeError('there is no context current')
         cdef alure.Context alure_context = (<Context> context).impl
         self.slot = alure_context.create_auxiliary_effect_slot()
         self.impl = alure_context.create_effect()
@@ -2449,6 +2453,7 @@ cdef class Decoder:
 
     def __init__(self, name: str, context: Optional[Context] = None) -> None:
         if context is None: context = current_context()
+        if not context: raise RuntimeError('there is no context current')
         self.pimpl = (<Context> context).impl.create_decoder(name)
 
     @getter

tests/functional/test_context_management.py

@@ -0,0 +1,79 @@
+# Context managers' functional tests
+# Copyright (C) 2020  Ngô Ngọc Đức Huy
+# Copyright (C) 2020  Nguyễn Gia Phong
+#
+# This file is part of palace.
+#
+# palace is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published
+# by the Free Software Foundation, either version 3 of the License,
+# or (at your option) any later version.
+#
+# palace is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with palace.  If not, see <https://www.gnu.org/licenses/>.
+
+from palace import (current_context, cache, free, decode, Device, Context,
+                    Buffer, Source, SourceGroup, ReverbEffect, ChorusEffect)
+from pytest import mark, raises
+
+
+def test_current_context():
+    """Test the current context."""
+    with Device() as device, Context(device) as context:
+        assert current_context() == context
+    assert current_context() is None
+
+
+def test_stream_loading(wav):
+    """Test implication of context during stream loading."""
+    with Device() as device, Context(device): decode(wav)
+    with raises(RuntimeError): decode(wav)
+
+
+@mark.skip(reason='deadlock (GH-73)')
+def test_cache_and_free(aiff, flac, ogg):
+    """Test cache and free, with and without a current context."""
+    with Device() as device, Context(device):
+        cache([aiff, flac, ogg])
+        free([aiff, flac, ogg])
+    with raises(RuntimeError): cache([aiff, flac, ogg])
+    with raises(RuntimeError): free([aiff, flac, ogg])
+
+
+def test_buffer_loading(mp3):
+    """Test implication of context during buffer loading."""
+    with Device() as device, Context(device):
+        with Buffer(mp3): pass
+    with raises(RuntimeError):
+        with Buffer(mp3): pass
+
+
+@mark.parametrize('cls', [Source, SourceGroup, ReverbEffect, ChorusEffect])
+def test_init_others(cls):
+    """Test implication of context during object initialization."""
+    with Device() as device, Context(device):
+        with cls(): pass
+    with raises(RuntimeError):
+        with cls(): pass
+
+
+@mark.parametrize('data', [
+    'air_absorption_factor', 'cone_angles', 'distance_range', 'doppler_factor',
+    'gain', 'gain_auto', 'gain_range', 'group', 'looping', 'offset',
+    'orientation', 'outer_cone_gains', 'pitch', 'position', 'radius',
+    'relative', 'rolloff_factors', 'spatialize', 'stereo_angles', 'velocity'])
+def test_source_setter(data):
+    with Device() as device, Context(device): source = Source()
+    with raises(RuntimeError): setattr(source, data, getattr(source, data))
+
+
+def test_nested_context_manager():
+    """Test if the context manager returns to the previous context."""
+    with Device() as device, Context(device) as context:
+        with Context(device): pass
+        assert current_context() == context

tests/unit/test_context.py

@@ -26,21 +26,29 @@ from pytest import raises
 from math import inf
 
 
-def test_with_context(device):
+def test_comparison(device):
-    """Test if `with` can be used to start a context
+    """Test basic comparisons."""
-    and is destroyed properly.
+    with Context(device) as c0, Context(device) as c1, Context(device) as c2:
-    """
+        assert c0 != c1
-    with Context(device) as context:
+        contexts = [c1, c1, c0, c2]
-        assert current_context() == context
+        contexts.sort()
+        contexts.remove(c2)
+        contexts.remove(c0)
+        assert contexts[0] == contexts[1]
 
 
-def test_nested_context_manager(device):
+def test_bool(device):
-    """Test if the context manager returns to the
+    """Test boolean value."""
-    previous context.
+    with Context(device) as context: assert context
-    """
+    assert not context
+
+
+def test_batch_control(device):
+    """Test calls of start_batch and end_batch."""
     with Context(device) as context:
-        with Context(device): pass
+        # At the moment these are no-op.
-        assert current_context() == context
+        context.start_batch()
+        context.end_batch()
 
 
 def test_message_handler(device):
@@ -61,36 +69,48 @@ def test_async_wake_interval(device):
         assert context.async_wake_interval == 42
 
 
+def test_format_support(device):
+    """Test method is_supported."""
+    with Context(device) as context:
+        assert isinstance(context.is_supported('Rear', '32-bit float'), bool)
+        with raises(ValueError): context.is_supported('Shu', 'Mulaw')
+        with raises(ValueError): context.is_supported('Stereo', 'Type')
+
+
 def test_default_resampler_index(device):
-    """Test return values default_resampler_index."""
+    """Test read-only property default_resampler_index."""
     with Context(device) as context:
         index = context.default_resampler_index
         assert index >= 0
         assert len(context.available_resamplers) > index
+        with raises(AttributeError): context.available_resamplers = 0
 
 
 def test_doppler_factor(device):
-    """Test write property doppler_factor."""
+    """Test write-only property doppler_factor."""
     with Context(device) as context:
         context.doppler_factor = 4/9
         context.doppler_factor = 9/4
         context.doppler_factor = 0
         context.doppler_factor = inf
         with raises(ValueError): context.doppler_factor = -1
+        with raises(AttributeError): context.doppler_factor
 
 
 def test_speed_of_sound(device):
-    """Test write property speed_of_sound."""
+    """Test write-only property speed_of_sound."""
     with Context(device) as context:
         context.speed_of_sound = 5/7
         context.speed_of_sound = 7/5
         with raises(ValueError): context.speed_of_sound = 0
         context.speed_of_sound = inf
         with raises(ValueError): context.speed_of_sound = -1
+        with raises(AttributeError): context.speed_of_sound
 
 
 def test_distance_model(device):
-    """Test preset values distance_model."""
+    """Test write-only distance_model."""
     with Context(device) as context:
         for model in distance_models: context.distance_model = model
         with raises(ValueError): context.distance_model = 'EYYYYLMAO'
+        with raises(AttributeError): context.distance_model

tests/unit/test_listener.py

@@ -1,5 +1,6 @@
 # Listener pytest module
 # Copyright (C) 2020  Ngô Xuân Minh
+# Copyright (C) 2020  Nguyễn Gia Phong
 #
 # This file is part of palace.
 #
@@ -18,54 +19,52 @@
 
 """This pytest module tries to test the correctness of the class Listener."""
 
-from pytest import raises
+from pytest import mark, raises
 
 from math import inf
 
 
 def test_gain(context):
-    """Test write property gain."""
+    """Test write-only property gain."""
     context.listener.gain = 5/7
     context.listener.gain = 7/5
     context.listener.gain = 0
     context.listener.gain = inf
     with raises(ValueError): context.listener.gain = -1
+    with raises(AttributeError): context.listener.gain
 
 
-def test_position(context):
+@mark.parametrize('position', [(1, 0, 1), (1, 0, -1), (1, -1, 0),
-    """Test write property position."""
+                               (1, 1, 0), (0, 0, 0), (1, 1, 1)])
-    context.listener.position = 1, 0, 1
+def test_position(context, position):
-    context.listener.position = 1, 0, -1
+    """Test write-only property position."""
-    context.listener.position = 1, -1, 0
+    context.listener.position = position
-    context.listener.position = 1, 1, 0
+    with raises(AttributeError): context.listener.position
-    context.listener.position = 0, 0, 0
-    context.listener.position = 1, 1, 1
 
 
-def test_velocity(context):
+@mark.parametrize('velocity', [(420, 0, 69), (69, 0, -420), (0, 420, -69),
-    """Test write property velocity."""
+                               (0, 0, 42), (0, 0, 0), (420, 69, 420)])
-    context.listener.velocity = 420, 0, 69
+def test_velocity(context, velocity):
-    context.listener.velocity = 69, 0, -420
+    """Test write-only property velocity."""
-    context.listener.velocity = 0, 420, -69
+    context.listener.velocity = velocity
-    context.listener.velocity = 0, 0, 42
+    with raises(AttributeError): context.listener.velocity
-    context.listener.velocity = 0, 0, 0
-    context.listener.velocity = 420, 69, 420
 
 
-def test_orientaion(context):
+@mark.parametrize(('at', 'up'), [
-    """Test write property orientation."""
+    ((420, 0, 69), (0, 42, 0)), ((69, 0, -420), (0, -69, 420)),
-    context.listener.orientation = (420, 0, 69), (0, 42, 0)
+    ((0, 420, -69), (420, -69, 69)), ((0, 0, 42), (-420, -420, 0)),
-    context.listener.orientation = (69, 0, -420), (0, -69, 420)
+    ((0, 0, 0), (-420, -69, -69)), ((420, 69, 420), (69, -420, 0))])
-    context.listener.orientation = (0, 420, -69), (420, -69, 69)
+def test_orientaion(context, at, up):
-    context.listener.orientation = (0, 0, 42), (-420, -420, 0)
+    """Test write-only property orientation."""
-    context.listener.orientation = (0, 0, 0), (-420, -69, -69)
+    context.listener.orientation = at, up
-    context.listener.orientation = (420, 69, 420), (69, -420, 0)
+    with raises(AttributeError): context.listener.orientation
 
 
 def test_meters_per_unit(context):
-    """Test write property meter_per_unit."""
+    """Test write-only property meters_per_unit."""
     context.listener.meters_per_unit = 4/9
     context.listener.meters_per_unit = 9/4
     with raises(ValueError): context.listener.meters_per_unit = 0
     context.listener.meters_per_unit = inf
     with raises(ValueError): context.listener.meters_per_unit = -1
+    with raises(AttributeError): context.listener.meters_per_unit

tests/unit/test_source.py

@@ -60,6 +60,8 @@ def test_control(context, flac):
         source.stop()
         assert not source.playing
         assert not source.paused
+        with raises(AttributeError): source.playing = True
+        with raises(AttributeError): source.paused = True
 
 
 def test_fade_out_to_stop(context, mp3):
@@ -99,6 +101,30 @@ def test_offset(context, ogg):
         with raises(OverflowError): source.offset = -1
 
 
+def test_offset_seconds(context, flac):
+    """Test read-only property offset_seconds."""
+    with Buffer(flac) as buffer, buffer.play() as source:
+        assert isinstance(source.offset_seconds, float)
+        with raises(AttributeError):
+            source.offset_seconds = buffer.length_seconds / 2
+
+
+def test_latency(context, aiff):
+    """Test read-only property latency."""
+    with Buffer(aiff) as buffer, buffer.play() as source:
+        assert isinstance(source.latency, int)
+        with raises(AttributeError):
+            source.latency = 42
+
+
+def test_latency_seconds(context, mp3):
+    """Test read-only property latency_seconds."""
+    with Buffer(mp3) as buffer, buffer.play() as source:
+        assert isinstance(source.latency_seconds, float)
+        with raises(AttributeError):
+            source.latency_seconds = buffer.length_seconds / 2
+
+
 def test_looping(context):
     """Test read-write property looping."""
     with Source(context) as source:
@@ -300,15 +326,19 @@ def tests_sends(device, context):
             source.sends[i].filter = random(), random(), random()
             shuffle(invalid_filter)
             with raises(ValueError): source.sends[i].filter = invalid_filter
+            with raises(AttributeError): source.sends[i].effect
+            with raises(AttributeError): source.sends[i].filter
         with raises(IndexError): source.sends[-1]
         with raises(TypeError): source.sends[4.2]
         with raises(TypeError): source.sends['0']
         with raises(TypeError): source.sends[6:9]
+        with raises(AttributeError): source.sends = ...
 
 
 def test_filter(context):
     """Test write-only property filter."""
     with Source() as source:
+        with raises(AttributeError): source.filter
         source.filter = 1, 6.9, 5/7
         source.filter = 0, 0, 0
         for gain, gain_hf, gain_lf in permutations([4, -2, 0]):