From b669bb0327527c23dc481dec0c803ebc4ab53239 Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Thu, 7 May 2020 00:23:07 -0400
Subject: [PATCH] validate: Add support for known-issues in the .validatetest
And add some tests about remaining actions failures
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-devtools/-/merge_requests/189>
---
docs/gst-validate-test-file.md | 38 +++++++++-
validate/gst/validate/gst-validate-internal.h | 1 +
validate/gst/validate/gst-validate-report.c | 2 +
validate/gst/validate/gst-validate-report.h | 1 +
validate/gst/validate/gst-validate-runner.c | 74 +++++++++++++++++++
validate/gst/validate/validate.c | 44 ++++++++---
validate/launcher/baseclasses.py | 6 ++
...k_set_prop_never_called_error.validatetest | 9 +++
...egotiated.accept_caps_failure.validatetest | 7 ++
.../tests/launcher_tests/test_validate.py | 9 +++
validate/tools/gst-validate.c | 3 +-
11 files changed, 180 insertions(+), 14 deletions(-)
create mode 100644 validate/tests/launcher_tests/check_set_prop_never_called_error.validatetest
create mode 100644 validate/tests/launcher_tests/not_negotiated.accept_caps_failure.validatetest
diff --git a/docs/gst-validate-test-file.md b/docs/gst-validate-test-file.md
index 6328aa0ab7..045b78233a 100644
--- a/docs/gst-validate-test-file.md
+++ b/docs/gst-validate-test-file.md
@@ -44,8 +44,8 @@ args = {
## configs
-The `config` field is an array of string containing the same content as
-usual [config](gst-validate-config.md) files contain.
+The `configs` field is an array of structures containing the same content as
+usual [configs](gst-validate-config.md) files.
For example:
@@ -57,6 +57,40 @@ configs = {
}
```
+Note: Since this is GstStructure synthax, we need to have the structures in the
+array as strings/within quotes.
+
+## expected-issues
+
+The `expected-issues` field is an array of `expected-issue` structures containing
+information about issues to expect (which can be known bugs or not).
+
+Use `gst-validate-1.0 --print-issue-types` to print information about all issue types.
+
+For example:
+
+``` yaml
+expected-issues = {
+ "expected-issue, issue-id=scenario::not-ended",
+}
+```
+
+Note: Since this is GstStructure synthax, we need to have the structures in the
+array as strings/within quotes.
+
+### Fields:
+
+* `issue-id`: (string): Issue ID - Mandatory if `summary` is not provided.
+* `summary`: (string): Summary - Mandatory if `issue-id` is not provided.
+* `details`: Regex string to match the issue details `detected-on`: (string):
+ The name of the element the issue happened on `level`: (string):
+ Issue level
+* `sometimes`: (boolean): Default: `false` - Wheteher the issue happens only
+ sometimes if `false` and the issue doesn't happen, an error will
+ be issued.
+* `issue-url`: (string): The url of the issue in the bug tracker if the issue is
+ a bug.
+
# Variables
The same way
diff --git a/validate/gst/validate/gst-validate-internal.h b/validate/gst/validate/gst-validate-internal.h
index 07912a2164..8fc4880494 100644
--- a/validate/gst/validate/gst-validate-internal.h
+++ b/validate/gst/validate/gst-validate-internal.h
@@ -63,4 +63,5 @@ G_GNUC_INTERNAL gboolean gst_validate_get_test_file_scenario (GList** structs, c
G_GNUC_INTERNAL GstValidateScenario* gst_validate_scenario_from_structs (GstValidateRunner* runner, GstElement* pipeline, GList* structures,
gchar* origin_file);
G_GNUC_INTERNAL GList* gst_validate_get_config(const gchar *structname);
+G_GNUC_INTERNAL GList * gst_validate_get_test_file_expected_issues (void);
#endif
diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c
index 2372c7e0bd..b8a4be2a16 100644
--- a/validate/gst/validate/gst-validate-report.c
+++ b/validate/gst/validate/gst-validate-report.c
@@ -677,6 +677,8 @@ gst_validate_report_level_get_name (GstValidateReportLevel level)
return "issue";
case GST_VALIDATE_REPORT_LEVEL_IGNORE:
return "ignore";
+ case GST_VALIDATE_REPORT_LEVEL_EXPECTED:
+ return "expected";
default:
return "unknown";
}
diff --git a/validate/gst/validate/gst-validate-report.h b/validate/gst/validate/gst-validate-report.h
index 1643b1b911..f19e8f2ccc 100644
--- a/validate/gst/validate/gst-validate-report.h
+++ b/validate/gst/validate/gst-validate-report.h
@@ -67,6 +67,7 @@ typedef enum {
GST_VALIDATE_REPORT_LEVEL_ISSUE,
GST_VALIDATE_REPORT_LEVEL_IGNORE,
GST_VALIDATE_REPORT_LEVEL_UNKNOWN,
+ GST_VALIDATE_REPORT_LEVEL_EXPECTED,
GST_VALIDATE_REPORT_LEVEL_NUM_ENTRIES,
} GstValidateReportLevel;
diff --git a/validate/gst/validate/gst-validate-runner.c b/validate/gst/validate/gst-validate-runner.c
index 06d0a0f31d..53ab4c566b 100644
--- a/validate/gst/validate/gst-validate-runner.c
+++ b/validate/gst/validate/gst-validate-runner.c
@@ -93,6 +93,8 @@ struct _GstValidateRunnerPrivate
gchar *pipeline_names;
gchar **pipeline_names_strv;
+
+ GList *expected_issues;
};
/* Describes the reporting level to apply to a name pattern */
@@ -448,6 +450,8 @@ gst_validate_runner_init (GstValidateRunner * runner)
runner->priv->default_level = GST_VALIDATE_SHOW_DEFAULT;
_init_report_levels (runner);
+ runner->priv->expected_issues = gst_validate_get_test_file_expected_issues ();
+
gst_tracing_register_hook (GST_TRACER (runner), "element-new",
G_CALLBACK (do_element_new));
@@ -629,6 +633,50 @@ gst_validate_runner_maybe_dot_pipeline (GstValidateRunner * runner,
}
}
+static gboolean
+check_report_expected (GstValidateRunner * runner, GstValidateReport * report)
+{
+ GList *tmp;
+
+#define GET_STR(name) gst_structure_get_string (known_issue, name)
+
+ for (tmp = runner->priv->expected_issues; tmp; tmp = tmp->next) {
+ GstStructure *known_issue = tmp->data;
+ const gchar *id = GET_STR ("issue-id");
+
+ if (!id || g_quark_from_string (id) == report->issue->issue_id) {
+ const gchar *summary = GET_STR ("summary");
+
+ if (!summary || !g_strcmp0 (summary, report->issue->summary)) {
+ const gchar *details = GET_STR ("details");
+
+ if (!details || g_regex_match_simple (details, report->message, 0, 0)) {
+ const gchar *detected_on = GET_STR ("detected-on");
+
+ if (!detected_on || !g_strcmp0 (detected_on, report->reporter_name)) {
+ const gchar *level = GET_STR ("level");
+ const gchar *report_level =
+ gst_validate_report_level_get_name (report->level);
+
+ if (!detected_on || !g_strcmp0 (level, report_level)) {
+ gboolean is_sometimes;
+
+ if (!gst_structure_get_boolean (known_issue, "sometimes",
+ &is_sometimes) || !is_sometimes)
+ runner->priv->expected_issues =
+ g_list_remove (runner->priv->expected_issues, known_issue);
+ return TRUE;
+ }
+ }
+ }
+ }
+ }
+#undef GET_STR
+ }
+
+ return FALSE;
+}
+
void
gst_validate_runner_add_report (GstValidateRunner * runner,
GstValidateReport * report)
@@ -640,6 +688,9 @@ gst_validate_runner_add_report (GstValidateRunner * runner,
if (report->level == GST_VALIDATE_REPORT_LEVEL_IGNORE)
return;
+ if (check_report_expected (runner, report))
+ report->level = GST_VALIDATE_REPORT_LEVEL_EXPECTED;
+
gst_validate_send (json_boxed_serialize (GST_MINI_OBJECT_TYPE (report),
report));
gst_validate_runner_maybe_dot_pipeline (runner, report);
@@ -841,7 +892,10 @@ int
gst_validate_runner_exit (GstValidateRunner * runner, gboolean print_result)
{
gint ret = 0;
+ GList *tmp;
+
g_return_val_if_fail (GST_IS_VALIDATE_RUNNER (runner), 1);
+
g_signal_emit (runner, _signals[STOPPING_SIGNAL], 0);
if (print_result) {
ret = gst_validate_runner_printf (runner);
@@ -854,6 +908,26 @@ gst_validate_runner_exit (GstValidateRunner * runner, gboolean print_result)
}
}
+ for (tmp = runner->priv->expected_issues; tmp; tmp = tmp->next) {
+ GstStructure *known_issue = tmp->data;
+ gboolean is_sometimes;
+
+ if (!gst_structure_get_boolean (known_issue, "sometimes", &is_sometimes)
+ || !is_sometimes) {
+ GstStructure *tmp = gst_structure_copy (known_issue);
+ gst_structure_remove_fields (tmp, "__debug__", "__lineno__",
+ "__filename__", NULL);
+ /* Ideally we should report an issue here.. but we do not have a reporter */
+ gst_validate_error_structure (known_issue,
+ "Expected issue didn't happen: '%" GST_PTR_FORMAT "'", tmp);
+ gst_structure_free (tmp);
+ }
+ }
+
+ g_list_free_full (runner->priv->expected_issues,
+ (GDestroyNotify) gst_structure_free);
+ runner->priv->expected_issues = NULL;
+
return ret;
}
diff --git a/validate/gst/validate/validate.c b/validate/gst/validate/validate.c
index fa4e437fd7..434275dcef 100644
--- a/validate/gst/validate/validate.c
+++ b/validate/gst/validate/validate.c
@@ -236,10 +236,10 @@ create_config (const gchar * config, const gchar * suffix)
}
static GList *
-gst_validate_get_testfile_configs (const gchar * suffix)
+get_structures_from_array_in_meta (const gchar * fieldname)
{
GList *res = NULL;
- gchar **config_strs = NULL, *filename = NULL, *debug = NULL;
+ gchar **strs = NULL, *filename = NULL, *debug = NULL;
gint current_lineno = -1;
GstStructure *meta = get_test_file_meta ();
@@ -250,18 +250,17 @@ gst_validate_get_testfile_configs (const gchar * suffix)
"__lineno__", G_TYPE_INT, ¤t_lineno,
"__debug__", G_TYPE_STRING, &debug,
"__filename__", G_TYPE_STRING, &filename, NULL);
- config_strs = gst_validate_utils_get_strv (meta, "configs");
+ strs = gst_validate_utils_get_strv (meta, fieldname);
- if (config_strs) {
+ if (strs) {
gint i;
- for (i = 0; config_strs[i]; i++) {
- GstStructure *tmpstruct =
- gst_structure_from_string (config_strs[i], NULL);
+ for (i = 0; strs[i]; i++) {
+ GstStructure *tmpstruct = gst_structure_from_string (strs[i], NULL);
if (tmpstruct == NULL) {
gst_validate_abort ("%s:%d: Invalid structure\n %4d | %s\n%s",
- filename, current_lineno, current_lineno, config_strs[i], debug);
+ filename, current_lineno, current_lineno, strs[i], debug);
}
gst_structure_set (tmpstruct,
@@ -274,7 +273,15 @@ gst_validate_get_testfile_configs (const gchar * suffix)
g_free (filename);
g_free (debug);
- g_strfreev (config_strs);
+ g_strfreev (strs);
+
+ return res;
+}
+
+static GList *
+gst_validate_get_testfile_configs (const gchar * suffix)
+{
+ GList *res = get_structures_from_array_in_meta ("configs");
return get_config_from_structures (res, NULL, suffix);
}
@@ -481,6 +488,24 @@ gst_validate_is_initialized (void)
return validate_initialized;
}
+GList *
+gst_validate_get_test_file_expected_issues (void)
+{
+ GList *res = get_structures_from_array_in_meta ("expected-issues"), *tmp;
+
+ for (tmp = res; tmp; tmp = tmp->next) {
+ GstStructure *known_issue = tmp->data;
+ const gchar *summary = gst_structure_get_string (known_issue, "summary");
+ const gchar *id = gst_structure_get_string (known_issue, "issue-id");
+
+ if (!id && !summary)
+ gst_validate_error_structure (known_issue,
+ "Missing 'summary' or 'issue-id' fields.");
+ }
+
+ return res;
+}
+
gboolean
gst_validate_get_test_file_scenario (GList ** structs,
const gchar ** scenario_name, gchar ** original_name)
@@ -553,7 +578,6 @@ gst_validate_setup_test_file (const gchar * testfile, gboolean use_fakesinks)
gst_validate_scenario_check_and_set_needs_clock_sync (testfile_structs, &res);
gst_validate_set_test_file_globals (res, testfile, use_fakesinks);
-
gst_validate_structure_resolve_variables (res, NULL);
tool = gst_structure_get_string (res, "tool");
diff --git a/validate/launcher/baseclasses.py b/validate/launcher/baseclasses.py
index 3bf73cc0e0..bea38bdd21 100644
--- a/validate/launcher/baseclasses.py
+++ b/validate/launcher/baseclasses.py
@@ -1091,6 +1091,12 @@ class GstValidateTest(Test):
Colors.ENDC)
result = Result.KNOWN_ERROR
+ if result == Result.PASSED:
+ for report in self.reports:
+ if report["level"] == "expected":
+ result = Result.KNOWN_ERROR
+ break
+
self.set_result(result, msg.strip())
def _generate_expected_issues(self):
diff --git a/validate/tests/launcher_tests/check_set_prop_never_called_error.validatetest b/validate/tests/launcher_tests/check_set_prop_never_called_error.validatetest
new file mode 100644
index 0000000000..8bb73e14f1
--- /dev/null
+++ b/validate/tests/launcher_tests/check_set_prop_never_called_error.validatetest
@@ -0,0 +1,9 @@
+meta,
+ args = {
+ "fakesrc num-buffers=1 ! fakesink",
+ },
+ expected-issues = {
+ "expected-issue, issue-id=scenario::not-ended",
+ }
+
+set-property, target-element-factory-name=capsfilter, property-name=caps, property-value="video/x-raw,framerate=30/1,format=I420"
\ No newline at end of file
diff --git a/validate/tests/launcher_tests/not_negotiated.accept_caps_failure.validatetest b/validate/tests/launcher_tests/not_negotiated.accept_caps_failure.validatetest
new file mode 100644
index 0000000000..aa8777cabf
--- /dev/null
+++ b/validate/tests/launcher_tests/not_negotiated.accept_caps_failure.validatetest
@@ -0,0 +1,7 @@
+meta,
+ args = {
+ "audiotestsrc ! capsfilter caps=\"audio/x-raw,channels=2,channel-mask=(bitmask)0x67\" ! audioconvert ! capsfilter caps=\"audio/x-raw,channels=6,channel-mask=(bitmask)0x32\" name=capsfilter ! fakesink",
+ },
+ expected-issues = {
+ "expected-issue, level=critical, summary=\"a NOT NEGOTIATED message has been posted on the bus.\", details=\".*Caps negotiation failed at pad.*capsfilter:sink.*as it refused caps:.*\"",
+ }
diff --git a/validate/tests/launcher_tests/test_validate.py b/validate/tests/launcher_tests/test_validate.py
index 2591918baf..bad3275344 100644
--- a/validate/tests/launcher_tests/test_validate.py
+++ b/validate/tests/launcher_tests/test_validate.py
@@ -20,6 +20,8 @@
"""
The GstValidate default testsuite
"""
+import os
+from launcher.apps.gstvalidate import GstValidateSimpleTestsGenerator
TEST_MANAGER = "validate"
@@ -43,6 +45,8 @@ def get_pipelines(test_manager):
def setup_tests(test_manager, options):
+ testsuite_dir = os.path.realpath(os.path.join(os.path.dirname(__file__)))
+
print("Setting up tests to test GstValidate")
# No restriction about scenarios that are potentially used
valid_scenarios = ["play_15s"]
@@ -52,4 +56,9 @@ def setup_tests(test_manager, options):
pipelines_descriptions=get_pipelines(test_manager),
valid_scenarios=valid_scenarios))
+ test_manager.add_generators(
+ GstValidateSimpleTestsGenerator("simple", test_manager,
+ os.path.join(testsuite_dir))
+ )
+
return True
diff --git a/validate/tools/gst-validate.c b/validate/tools/gst-validate.c
index 968af6dd5e..e3b4373dcd 100644
--- a/validate/tools/gst-validate.c
+++ b/validate/tools/gst-validate.c
@@ -408,8 +408,7 @@ main (int argc, gchar ** argv)
"as gstreamer debugging");
if (argc == 1) {
- gst_validate_printf (NULL, "%s", g_option_context_get_help (ctx, FALSE,
- NULL));
+ g_print ("%s", g_option_context_get_help (ctx, FALSE, NULL));
exit (1);
}