From f4b30b08fedcee690dc9909baf2e431094156eed Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Tue, 14 Feb 2023 17:02:43 +0100 Subject: [PATCH 1/2] Remove assert_pipeline_play --- lib/membrane/testing/assertions.ex | 4 ---- test/membrane/core/pipeline_test.exs | 5 ++++- test/membrane/element_test.exs | 9 --------- test/membrane/integration/auto_demands_test.exs | 7 +------ test/membrane/integration/bin_test.exs | 6 ------ test/membrane/integration/child_crash_test.exs | 8 -------- test/membrane/integration/child_removal_test.exs | 2 -- test/membrane/integration/child_spawn_test.exs | 14 ++++++++------ test/membrane/integration/defer_setup_test.exs | 2 -- test/membrane/integration/demands_test.exs | 1 - .../integration/distributed_pipeline_test.exs | 2 +- .../integration/elements_compatibility_test.exs | 2 -- test/membrane/integration/endpoint_test.exs | 2 -- test/membrane/integration/linking_test.exs | 2 -- .../integration/no_stream_format_crash_test.exs | 2 +- test/membrane/integration/sync_test.exs | 1 - test/membrane/integration/timer_test.exs | 2 -- test/membrane/testing/dynamic_source_test.exs | 2 -- test/membrane/testing/pipeline_assertions_test.exs | 9 --------- test/membrane/testing/pipeline_test.exs | 2 -- 20 files changed, 15 insertions(+), 69 deletions(-) diff --git a/lib/membrane/testing/assertions.ex b/lib/membrane/testing/assertions.ex index 1da52d45e..7b394c40e 100644 --- a/lib/membrane/testing/assertions.ex +++ b/lib/membrane/testing/assertions.ex @@ -171,10 +171,6 @@ defmodule Membrane.Testing.Assertions do assert_receive_from_pipeline(pipeline, :setup, timeout) end - defmacro assert_pipeline_play(pipeline, timeout \\ @default_timeout) do - assert_receive_from_pipeline(pipeline, :play, timeout) - end - @doc """ Asserts that pipeline received or will receive a message matching `message_pattern` from another process within the `timeout` period specified diff --git a/test/membrane/core/pipeline_test.exs b/test/membrane/core/pipeline_test.exs index db093449e..7f9bb42a5 100644 --- a/test/membrane/core/pipeline_test.exs +++ b/test/membrane/core/pipeline_test.exs @@ -143,6 +143,9 @@ defmodule Membrane.Core.PipelineTest do } Testing.Pipeline.execute_actions(pid, spec: spec) - assert_pipeline_play(pid) + + for payload <- [1, 2, 3] do + assert_sink_buffer(pid, :b, %Membrane.Buffer{payload: ^payload}) + end end end diff --git a/test/membrane/element_test.exs b/test/membrane/element_test.exs index baf206417..c92347554 100644 --- a/test/membrane/element_test.exs +++ b/test/membrane/element_test.exs @@ -76,20 +76,15 @@ defmodule Membrane.ElementTest do end test "play", %{pipeline: pipeline} do - assert_pipeline_play(pipeline) TestFilter.assert_callback_called(:handle_playing) end describe "Start of stream" do test "causes handle_start_of_stream/3 to be called", %{pipeline: pipeline} do - assert_pipeline_play(pipeline) - TestFilter.assert_callback_called(:handle_start_of_stream) end test "does not trigger calling callback handle_event/3", %{pipeline: pipeline} do - assert_pipeline_play(pipeline) - TestFilter.refute_callback_called(:handle_event) end @@ -101,14 +96,10 @@ defmodule Membrane.ElementTest do describe "End of stream" do @tag :target test "causes handle_end_of_stream/3 to be called", %{pipeline: pipeline} do - assert_pipeline_play(pipeline) - TestFilter.assert_callback_called(:handle_end_of_stream) end test "does not trigger calling callback handle_event/3", %{pipeline: pipeline} do - assert_pipeline_play(pipeline) - TestFilter.refute_callback_called(:handle_event) end diff --git a/test/membrane/integration/auto_demands_test.exs b/test/membrane/integration/auto_demands_test.exs index f7e60d072..fd6914239 100644 --- a/test/membrane/integration/auto_demands_test.exs +++ b/test/membrane/integration/auto_demands_test.exs @@ -75,8 +75,6 @@ defmodule Membrane.Integration.AutoDemandsTest do |> child(:sink, Sink) ) - assert_pipeline_play(pipeline) - Enum.each(out_payloads, fn payload -> assert_sink_buffer(pipeline, :sink, buffer) assert buffer.payload == payload @@ -99,7 +97,6 @@ defmodule Membrane.Integration.AutoDemandsTest do ] ) - assert_pipeline_play(pipeline) Pipeline.message_child(pipeline, :right_sink, {:make_demand, 1000}) Enum.each(1..1000, fn payload -> @@ -124,7 +121,6 @@ defmodule Membrane.Integration.AutoDemandsTest do ] ) - assert_pipeline_play(pipeline) Process.sleep(500) Pipeline.execute_actions(pipeline, remove_children: :right_sink) @@ -215,7 +211,6 @@ defmodule Membrane.Integration.AutoDemandsTest do |> child(:sink, Sink) ) - assert_pipeline_play(pipeline) buffers = Enum.map(1..10, &%Membrane.Buffer{payload: &1}) Pipeline.message_child(pipeline, :source, buffer: {:output, buffers}) @@ -245,7 +240,7 @@ defmodule Membrane.Integration.AutoDemandsTest do ) Process.monitor(pipeline) - assert_pipeline_play(pipeline) + buffers = Enum.map(1..100_000, &%Membrane.Buffer{payload: &1}) Pipeline.message_child(pipeline, :source, buffer: {:output, buffers}) assert_receive({:DOWN, _ref, :process, ^pipeline, {:shutdown, :child_crash}}) diff --git a/test/membrane/integration/bin_test.exs b/test/membrane/integration/bin_test.exs index 6ef3486da..256d6866c 100644 --- a/test/membrane/integration/bin_test.exs +++ b/test/membrane/integration/bin_test.exs @@ -105,8 +105,6 @@ defmodule Membrane.Core.BinTest do pipeline = Testing.Pipeline.start_link_supervised!(spec: children) - assert_pipeline_play(pipeline) - assert_pipeline_notified(pipeline, :test_bin, {:handle_element_start_of_stream, :sink, _}) assert_buffers_flow_through(pipeline, buffers, :test_bin) @@ -127,8 +125,6 @@ defmodule Membrane.Core.BinTest do pipeline = Testing.Pipeline.start_link_supervised!(spec: links) - assert_pipeline_play(pipeline) - assert_pipeline_notified( pipeline, :test_bin, @@ -282,8 +278,6 @@ defmodule Membrane.Core.BinTest do end defp assert_data_flows_through(pipeline, buffers, receiving_element \\ :sink) do - assert_pipeline_play(pipeline) - assert_start_of_stream(pipeline, ^receiving_element) assert_buffers_flow_through(pipeline, buffers, receiving_element) diff --git a/test/membrane/integration/child_crash_test.exs b/test/membrane/integration/child_crash_test.exs index f397d5213..82de61a3e 100644 --- a/test/membrane/integration/child_crash_test.exs +++ b/test/membrane/integration/child_crash_test.exs @@ -29,8 +29,6 @@ defmodule Membrane.Integration.ChildCrashTest do ] |> Enum.map(&get_pid_and_link(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) - ChildCrashTest.Pipeline.crash_child(filter_1_1_pid) # assert all members of pipeline and pipeline itself down @@ -54,8 +52,6 @@ defmodule Membrane.Integration.ChildCrashTest do [{Membrane.Child, 1, :source}, :center_filter, :sink] |> Enum.map(&get_pid_and_link(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) - Process.exit(source_pid, :crash) # member of group is dead assert_pid_dead(source_pid) @@ -101,8 +97,6 @@ defmodule Membrane.Integration.ChildCrashTest do ] |> Enum.map(&get_pid_and_link(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) - filter_1_pid = get_pid(:filter, bin_1_pid) ChildCrashTest.Filter.crash(filter_1_pid) @@ -164,8 +158,6 @@ defmodule Membrane.Integration.ChildCrashTest do ] |> Enum.map(&get_pid_and_link(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) - ChildCrashTest.Pipeline.crash_child(filter_1_1_pid) # assert all members of group are dead diff --git a/test/membrane/integration/child_removal_test.exs b/test/membrane/integration/child_removal_test.exs index f0d91dab9..e498905d3 100644 --- a/test/membrane/integration/child_removal_test.exs +++ b/test/membrane/integration/child_removal_test.exs @@ -56,7 +56,6 @@ defmodule Membrane.Integration.ChildRemovalTest do [:filter1, :filter2, :filter3] |> Enum.map(&get_filter_pid(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) assert_pipeline_notified(pipeline_pid, :filter1, :playing) assert_pipeline_notified(pipeline_pid, :filter2, :playing) assert_pipeline_notified(pipeline_pid, :filter3, :playing) @@ -95,7 +94,6 @@ defmodule Membrane.Integration.ChildRemovalTest do [:filter1, :filter2, :filter3] |> Enum.map(&get_filter_pid(&1, pipeline_pid)) - assert_pipeline_play(pipeline_pid) assert_pipeline_notified(pipeline_pid, :filter1, :playing) assert_pipeline_notified(pipeline_pid, :filter2, :playing) assert_pipeline_notified(pipeline_pid, :filter3, :playing) diff --git a/test/membrane/integration/child_spawn_test.exs b/test/membrane/integration/child_spawn_test.exs index 0991fe504..1f736dd73 100644 --- a/test/membrane/integration/child_spawn_test.exs +++ b/test/membrane/integration/child_spawn_test.exs @@ -79,7 +79,7 @@ defmodule Membrane.Integration.ChildSpawnTest do |> child(:sink, SinkThatNotifiesParent, get_if_exists: true) Testing.Pipeline.execute_actions(pipeline_pid, spec: spec) - assert_pipeline_play(pipeline_pid) + refute_pipeline_notified(pipeline_pid, :sink, :message_from_sink) end @@ -95,8 +95,11 @@ defmodule Membrane.Integration.ChildSpawnTest do child(:source, %Testing.Source{output: [1, 2, 3]}) |> child(:sink, Testing.Sink, get_if_exists: true) - Testing.Pipeline.execute_actions(pipeline_pid, spec: spec, setup: :complete) - assert_pipeline_play(pipeline_pid) + Testing.Pipeline.execute_actions(pipeline_pid, spec: spec) + + for payload <- [1, 2, 3] do + assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: ^payload}) + end end test "if child/3 doesn't spawn child with a given name if there is already a child with given name among the children @@ -112,7 +115,7 @@ defmodule Membrane.Integration.ChildSpawnTest do |> child(:sink, Testing.Sink) Testing.Pipeline.execute_actions(pipeline_pid, spec: spec) - assert_pipeline_play(pipeline_pid) + assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 1}) assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 2}) assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 3}) @@ -134,7 +137,7 @@ defmodule Membrane.Integration.ChildSpawnTest do |> child(:sink, Testing.Sink, get_if_exists: true) Testing.Pipeline.execute_actions(pipeline_pid, spec: spec) - assert_pipeline_play(pipeline_pid) + assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 1}) assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 2}) assert_sink_buffer(pipeline_pid, :sink, %Buffer{payload: 3}) @@ -193,7 +196,6 @@ defmodule Membrane.Integration.ChildSpawnTest do pipeline_pid = Testing.Pipeline.start_supervised!() spec = child(%Testing.Source{output: [1, 2, 3]}) |> child(Testing.Sink) Testing.Pipeline.execute_actions(pipeline_pid, spec: spec) - assert_pipeline_play(pipeline_pid) Testing.Pipeline.terminate(pipeline_pid) end diff --git a/test/membrane/integration/defer_setup_test.exs b/test/membrane/integration/defer_setup_test.exs index d66983f57..bf40457dd 100644 --- a/test/membrane/integration/defer_setup_test.exs +++ b/test/membrane/integration/defer_setup_test.exs @@ -93,8 +93,6 @@ defmodule Membrane.Integration.DeferSetupTest do |> child(:bin_2, %Bin{defer_play: false}) ) - assert_pipeline_play(pipeline) - for bin <- [:bin_1, :bin_2] do refute_child_playing(pipeline, bin) end diff --git a/test/membrane/integration/demands_test.exs b/test/membrane/integration/demands_test.exs index a650a58eb..43fb0eca4 100644 --- a/test/membrane/integration/demands_test.exs +++ b/test/membrane/integration/demands_test.exs @@ -18,7 +18,6 @@ defmodule Membrane.Integration.DemandsTest do defp test_pipeline(pid) do pattern_gen = fn i -> %Buffer{payload: <> <> <<255>>} end - assert_pipeline_play(pid) demand = 500 Pipeline.message_child(pid, :sink, {:make_demand, demand}) diff --git a/test/membrane/integration/distributed_pipeline_test.exs b/test/membrane/integration/distributed_pipeline_test.exs index dd42bddcf..6204b8ee7 100644 --- a/test/membrane/integration/distributed_pipeline_test.exs +++ b/test/membrane/integration/distributed_pipeline_test.exs @@ -27,7 +27,7 @@ defmodule Membrane.Integration.DistributedPipelineTest do end pipeline = Membrane.Testing.Pipeline.start_link_supervised!(module: Pipeline) - assert_pipeline_play(pipeline) + assert_end_of_stream(pipeline, :sink) end diff --git a/test/membrane/integration/elements_compatibility_test.exs b/test/membrane/integration/elements_compatibility_test.exs index 7c95d6def..4105210cf 100644 --- a/test/membrane/integration/elements_compatibility_test.exs +++ b/test/membrane/integration/elements_compatibility_test.exs @@ -245,8 +245,6 @@ defmodule Membrane.Integration.ElementsCompatibilityTest do |> child(sink_module.__struct__(test_pid: self())) ) - assert_pipeline_play(pid) - state_dump = receive do {:state_dump, state_dump} -> state_dump diff --git a/test/membrane/integration/endpoint_test.exs b/test/membrane/integration/endpoint_test.exs index d6af1d801..a28baf353 100644 --- a/test/membrane/integration/endpoint_test.exs +++ b/test/membrane/integration/endpoint_test.exs @@ -49,8 +49,6 @@ defmodule Membrane.Core.EndpointTest do end defp assert_data_flows_through(pipeline, buffers, receiving_element) do - assert_pipeline_play(pipeline) - assert_start_of_stream(pipeline, ^receiving_element) buffers diff --git a/test/membrane/integration/linking_test.exs b/test/membrane/integration/linking_test.exs index d6707205f..d063cfd7a 100644 --- a/test/membrane/integration/linking_test.exs +++ b/test/membrane/integration/linking_test.exs @@ -171,7 +171,6 @@ defmodule Membrane.Integration.LinkingTest do source_pid = get_child_pid(:source, bin_pid) source_ref = Process.monitor(source_pid) - assert_pipeline_play(pipeline) Process.exit(sink_pid, :kill) assert_pipeline_crash_group_down(pipeline, :group_2) @@ -286,7 +285,6 @@ defmodule Membrane.Integration.LinkingTest do spec = [bin_spec, sink_spec, links_spec] send(pipeline, {:start_spec, %{spec: spec}}) assert_receive(:spec_started) - assert_pipeline_play(pipeline) end defmodule SlowSetupSink do diff --git a/test/membrane/integration/no_stream_format_crash_test.exs b/test/membrane/integration/no_stream_format_crash_test.exs index 93f536bac..62cad9023 100644 --- a/test/membrane/integration/no_stream_format_crash_test.exs +++ b/test/membrane/integration/no_stream_format_crash_test.exs @@ -52,7 +52,7 @@ defmodule Membrane.FailWhenNoStreamFormatAreSent do end source_ref = Process.monitor(source_pid) - assert_pipeline_play(pipeline) + Pipeline.message_child(pipeline, :source, :send_buffer) assert_receive {:DOWN, ^source_ref, :process, ^source_pid, {reason, _stack_trace}} assert %Membrane.ElementError{message: action_error_msg} = reason diff --git a/test/membrane/integration/sync_test.exs b/test/membrane/integration/sync_test.exs index 32fadff98..dd1fe6639 100644 --- a/test/membrane/integration/sync_test.exs +++ b/test/membrane/integration/sync_test.exs @@ -62,7 +62,6 @@ defmodule Membrane.Integration.SyncTest do pipeline = Testing.Pipeline.start_link_supervised!(options) - assert_pipeline_play(pipeline) send(pipeline, {:spawn_children, spec}) assert_start_of_stream(pipeline, :sink_a) diff --git a/test/membrane/integration/timer_test.exs b/test/membrane/integration/timer_test.exs index dc617eb14..84d0a648f 100644 --- a/test/membrane/integration/timer_test.exs +++ b/test/membrane/integration/timer_test.exs @@ -63,7 +63,6 @@ defmodule Membrane.Integration.TimerTest do custom_args: self() ) - assert_pipeline_play(pipeline) assert_pipeline_notified(pipeline, :element, :tick) assert_pipeline_notified(pipeline, :bin, :tick) assert_receive :pipeline_tick @@ -87,7 +86,6 @@ defmodule Membrane.Integration.TimerTest do test "Stopping timer with `:no_interval`" do pipeline = Testing.Pipeline.start_link_supervised!(spec: [child(:element, StopNoInterval)]) - assert_pipeline_play(pipeline) assert_pipeline_notified(pipeline, :element, :ok) Testing.Pipeline.terminate(pipeline) end diff --git a/test/membrane/testing/dynamic_source_test.exs b/test/membrane/testing/dynamic_source_test.exs index 41347bf4d..a95a87eab 100644 --- a/test/membrane/testing/dynamic_source_test.exs +++ b/test/membrane/testing/dynamic_source_test.exs @@ -39,7 +39,6 @@ defmodule Membrane.Testing.DynamicSourceTest do ] ) - assert_pipeline_play(pipeline) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: 'a'}) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: 'b'}) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: 'c'}) @@ -63,7 +62,6 @@ defmodule Membrane.Testing.DynamicSourceTest do ] ) - assert_pipeline_play(pipeline) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: <<0::16>>}) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: <<1::16>>}) assert_sink_buffer(pipeline, :sink_1, %Buffer{payload: <<2::16>>}) diff --git a/test/membrane/testing/pipeline_assertions_test.exs b/test/membrane/testing/pipeline_assertions_test.exs index e77d8e873..89c22c670 100644 --- a/test/membrane/testing/pipeline_assertions_test.exs +++ b/test/membrane/testing/pipeline_assertions_test.exs @@ -37,15 +37,6 @@ defmodule Membrane.Testing.PipelineAssertionsTest do end end - test "assert_pipeline_play works", %{state: state} do - Pipeline.handle_playing(context(), state) - assert_pipeline_play(self()) - - assert_raise ExUnit.AssertionError, fn -> - assert_pipeline_play(self(), 0) - end - end - describe "assert_pipeline_receive" do test "does not flunk when pipeline receives a message", %{state: state} do message = "I am an important message" diff --git a/test/membrane/testing/pipeline_test.exs b/test/membrane/testing/pipeline_test.exs index 7b2c92307..9e548edb7 100644 --- a/test/membrane/testing/pipeline_test.exs +++ b/test/membrane/testing/pipeline_test.exs @@ -123,8 +123,6 @@ defmodule Membrane.Testing.PipelineTest do pipeline = Pipeline.start_supervised!(spec: spec) - assert_pipeline_play(pipeline) - # getting children pids from pipeline for bin <- [:bin_1, :bin_2, :bin_3] do Pipeline.execute_actions(pipeline, notify_child: {bin, {:get_pid, bin}}) From cdbc020e8e14259ef76d86620503d9f6f6fdb0d5 Mon Sep 17 00:00:00 2001 From: "feliks.pobiedzinski@swmansion.com" Date: Tue, 14 Feb 2023 17:26:09 +0100 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d42993e8c..84e35fe66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * The flow control of the pad is now set with a single `:flow_control` option instead of `:mode` and `:demand_mode` options. * Remove _t suffix from types [#509](https://github.com/membraneframework/membrane_core/pull/509) * Implement automatic demands in Membrane Sinks and Endpoints. [#512](https://github.com/membraneframework/membrane_core/pull/512) + * Remove `assert_pipeline_play/2` from `Membrane.Testing.Assertions`. [#528](https://github.com/membraneframework/membrane_core/pull/528) ## 0.11.0 * Separate element_name and pad arguments in handle_element_{start, end}_of_stream signature [#219](https://github.com/membraneframework/membrane_core/issues/219)