tests: fix pipelines_parse_launch.delayed_link flakiness
authorMathieu Duponchelle <mathieu@centricular.com>
Fri, 13 Dec 2019 17:21:32 +0000 (18:21 +0100)
committerGStreamer Merge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Sat, 14 Dec 2019 11:25:20 +0000 (11:25 +0000)
Fixes #345

There were two causes for the flakiness, one much rarer than
the other.

The test sets up a source with a sometimes pad added during
the transition of a wrapper bin from READY to PAUSED.

It runs 4 iterations, the last of which makes it so the
negotiation fails.

In that case, the intention as correctly presented by the following
comment:

/* [..] ie, the pipeline should create ok but fail to change state */

However the implementation of run_delayed_test was neither calling
get_state on the pipeline (it called it on the wrapper bin), nor
checking that the return of get_state was FAILURE (it actually
checked that it was not).

This led to an obvious race condition, and was fixed by calling
get_state on the pipeline, then checking that in this specific
case (expect_link == FALSE), the state change has actually failed.

The second, rarer race condition is at set_state time. When we
don't expect the link to succeed, the return of set_state may
either be FAILURE or ASYNC, depending on timing. This was fixed
by taking expect_link into account when checking the return value
of set_state.

Co-authored by: Thibault Saunier <tsaunier@igalia.com>

tests/check/pipelines/parse-launch.c

index b395ecd048c2656a925fd6b4e3ecf584ec55269c..ae7cb1787e1aa89ad628f3ed41864b4150fefb8c 100644 (file)
@@ -419,6 +419,7 @@ run_delayed_test (const gchar * pipe_str, const gchar * peer,
 {
   GstElement *pipe, *src, *sink;
   GstPad *srcpad, *sinkpad, *peerpad = NULL;
+  GstStateChangeReturn ret;
 
   pipe = setup_pipeline (pipe_str);
 
@@ -434,11 +435,22 @@ run_delayed_test (const gchar * pipe_str, const gchar * peer,
 
   /* Set the state to PAUSED and wait until the src at least reaches that
    * state */
-  fail_if (gst_element_set_state (pipe, GST_STATE_PAUSED) ==
-      GST_STATE_CHANGE_FAILURE);
+  ret = gst_element_set_state (pipe, GST_STATE_PAUSED);
+
+  if (expect_link) {
+    fail_if (ret == GST_STATE_CHANGE_FAILURE);
+  } else {
+    fail_unless (ret == GST_STATE_CHANGE_FAILURE
+        || ret == GST_STATE_CHANGE_ASYNC);
+  }
 
-  fail_if (gst_element_get_state (src, NULL, NULL, GST_CLOCK_TIME_NONE) ==
-      GST_STATE_CHANGE_FAILURE);
+  ret = gst_element_get_state (pipe, NULL, NULL, GST_CLOCK_TIME_NONE);
+
+  if (expect_link) {
+    fail_if (ret == GST_STATE_CHANGE_FAILURE);
+  } else {
+    fail_unless (ret == GST_STATE_CHANGE_FAILURE);
+  }
 
   /* Now, the source element should have a src pad, and if "peer" was passed, 
    * then the src pad should have gotten linked to the 'sink' pad of that