PR#4752: Use assertRegex instead of assertIn to avoid non-determinism failures - pagure

pagure

#4752 Use assertRegex instead of assertIn to avoid non-determinism failures

Merged 4 years ago by pingou. Opened 4 years ago by sergiodj.

sergiodj/pagure non-deterministic-tests-fix into master

Use assertRegex instead of assertIn to avoid non-determinism failures

Sergio Durigan Junior • 4 years ago

1c9abcd

tests/test_pagure_flask_ui_fork.py

file modified

+25 -38

		`@@ -54,6 +54,16 @@`
		`return commits`


		`+ MERGED_PATTERN = (`
		`+ re.escape('<span class="text-info font-weight-bold">Merged</span> ')`
		`+ + "(just now\|seconds ago)\n"`
		`+ + re.escape(`
		`+ " </span>\n by\n"`
		`+ ' <span title="PY C (pingou)">pingou.</span>\n'`
		`+ )`
		`+ )`
		`+`
		`+`
		`def set_up_git_repo(`
		`session,`
		`path,`
		`@@ -926,11 +936,8 @@`

		`# Check if the closing notification was added`
		`output = self.app.get("/test/pull-request/1")`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> just now\n'`
		`- " </span>\n by\n"`
		`- ' <span title="PY C (pingou)">pingou.</span>\n',`
		`- output.get_data(as_text=True),`
		`+ self.assertIsNotNone(`
		`+ re.search(MERGED_PATTERN, output.get_data(as_text=True))`
		`)`

		`@patch("pagure.lib.notify.send_email")`
		`@@ -972,11 +979,8 @@`

		`# Check if the closing notification was added`
		`output = self.app.get("/test/pull-request/1")`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> just now\n'`
		`- " </span>\n by\n"`
		`- ' <span title="PY C (pingou)">pingou.</span>\n',`
		`- output.get_data(as_text=True),`
		`+ self.assertIsNotNone(`
		`+ re.search(MERGED_PATTERN, output.get_data(as_text=True))`
		`)`

		`@patch("pagure.lib.notify.send_email")`
		`@@ -1028,12 +1032,7 @@`
		`# Check if the closing notification was added`
		`output = self.app.get("/test/pull-request/1")`
		`output_text = output.get_data(as_text=True)`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> just now\n'`
		`- " </span>\n by\n"`
		`- ' <span title="PY C (pingou)">pingou.</span>\n',`
		`- output_text,`
		`- )`
		`+ self.assertIsNotNone(re.search(MERGED_PATTERN, output_text))`
		`self.assertIn(`
		`"Thanks for the review and the suggestions!", output_text`
		`)`
		`@@ -1218,12 +1217,8 @@`

		`# Check if the closing notification was added`
		`output = self.app.get("/test/pull-request/1")`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> just now\n'`
		`- " </span>\n by\n"`
		`- ' <span title="PY C (pingou)">pingou.</span>\n',`
		`- output.get_data(as_text=True),`
		`- )`
		`+ output_text = output.get_data(as_text=True)`
		`+ self.assertIsNotNone(re.search(MERGED_PATTERN, output_text))`

		`@patch("pagure.lib.notify.send_email")`
		`def test_request_pull_close(self, send_email):`
		`@@ -1235,12 +1230,7 @@`
		`output = self.app.get("/test/pull-request/1")`
		`self.assertEqual(output.status_code, 200)`
		`output_text = output.get_data(as_text=True)`
		`-`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> '`
		`- "just now\n </span>\n by\n",`
		`- output_text,`
		`- )`
		`+ self.assertIsNotNone(re.search(MERGED_PATTERN, output_text))`
		`self.assertIn(`
		`'title="View file as of 2a552b">sources</a>', output_text`
		`)`
		`@@ -4348,11 +4338,12 @@`
		`output_text,`
		`)`
		`# Checking if Edited by User is there or not`
		`- self.assertTrue(`
		`- "<small>Edited just now by pingou </small>" in output_text`
		`- or "<small>Edited seconds ago by pingou </small>"`
		`- in output_text`
		`+ pattern = (`
		`+ re.escape("<small>Edited ")`
		`+ + "(just now\|seconds ago)"`
		`+ + re.escape(" by pingou </small>")`
		`)`
		`+ self.assertIsNotNone(re.search(pattern, output_text))`
		`self.assertIn("Comment updated", output_text)`

		`# Project w/o pull-request`
		`@@ -4466,12 +4457,8 @@`

		`# Check if the closing notification was added`
		`output = self.app.get("/test/pull-request/1")`
		`- self.assertIn(`
		`- '<span class="text-info font-weight-bold">Merged</span> just now\n'`
		`- " </span>\n by\n"`
		`- ' <span title="PY C (pingou)">pingou.</span>\n',`
		`- output.get_data(as_text=True),`
		`- )`
		`+ output_text = output.get_data(as_text=True)`
		`+ self.assertIsNotNone(re.search(MERGED_PATTERN, output_text))`

		`@patch("pagure.lib.notify.send_email")`
		`def test_internal_endpoint_main_ahead(self, send_email):`

sergiodj commented 4 years ago

Depending on the system load, we might see messages containing "just
now" or "seconds ago". For example:

"Merged just now..."

vs.

"Merged seconds ago..."

If we only look for "just now", we are obviously going to fail when
the system takes some time to process a request, and "seconds ago" is
displayed. This commit addresses this problem by using a regex when
matching the text, and expecting both patterns.

Fixes #4751

Signed-off-by: Sergio Durigan Junior sergiodj@sergiodj.net

rebased onto 9f4670f625a9c05156e018b43063657f5074460a

4 years ago

rebased onto a58b4bc18d4556cff93b2ad0c6bb919e5bf25bd8

4 years ago

rebased onto 6ae1910692211613a5f9f1168d1a2e0a9f1b4224

4 years ago

pingou commented 4 years ago

AttributeError: 'PagureFlaskForktests' object has no attribute 'assertRegex'

Looks like the unit-test version in CentOS7 is too old :(

rebased onto ea7bf28d834bf69bebc72752a05f9e17ff757b71

4 years ago

rebased onto 27df61ccbe36adf7bc6dba7966bd44d6ebfd3da4

4 years ago

pingou commented 4 years ago

@sergiodj I've adjusted the code to rely on re.search which according to the doc is what assertRegex does, this way we should be compatible with the unit-tests version that is in CentOS/RHEL7.