From 25d7914395f8483d1e79e8d4c657d00a80dc8b0b Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Fri, 13 Dec 2019 18:21:32 +0100 Subject: [PATCH] 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 --- tests/check/pipelines/parse-launch.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/check/pipelines/parse-launch.c b/tests/check/pipelines/parse-launch.c index b395ecd048..ae7cb1787e 100644 --- a/tests/check/pipelines/parse-launch.c +++ b/tests/check/pipelines/parse-launch.c @@ -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