tests: fix pipelines_parse_launch.delayed_link flakiness

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>
This commit is contained in:
Mathieu Duponchelle 2019-12-13 18:21:32 +01:00 committed by GStreamer Merge Bot
parent 4a827e11b4
commit 25d7914395

View 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);
fail_if (gst_element_get_state (src, NULL, NULL, GST_CLOCK_TIME_NONE) ==
GST_STATE_CHANGE_FAILURE);
if (expect_link) {
fail_if (ret == GST_STATE_CHANGE_FAILURE);
} else {
fail_unless (ret == GST_STATE_CHANGE_FAILURE
|| ret == GST_STATE_CHANGE_ASYNC);
}
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