Random test fail in SimplenewsSendTest

Let's try if testbot also fails?

And if this doesn't fail, we should try to encode it, it might get decoded accidentally somewhere.

Retriggering tests...
For reference, these are the previous failing runs:
https://www.drupal.org/pift-ci-job/509973
https://www.drupal.org/pift-ci-job/509871

Hm, the SQLite just failed 3 out of 4 times...
The others pass mostly passed, only mysql with 5.5 and 8.3 failed once.
Still strange, i was hoping to find fully consistent behavior...

Those are two different kinds of fails. The second one actually has a @todo related to https://www.drupal.org/node/2575791, so maybe the strip tags behavior is not consistent.

2 years on, the details of which tests are failing are probably different, but the problem still exists. Here is a patch that should solve the two that failed in the most recent run https://www.drupal.org/pift-ci-job/1192925.

1) SimplenewsAdministrationTest: the user is displayed as a link, so fix the tests to match (presumably this one fails always, not just randomly).

2) SimplenewsSourceTest: the problem comes from different HTML encodings, in particular whether to encode quotes. The mail text has legitimately chosen not to (because it isn't strictly necessary); HTML::encode encodes them anyway (because that's a safe thing to do). So comparing encoded values is not a good idea because there are many different alternative correct encodings. The solution is to compare decoded values rather than encoded values.

+++ b/src/Tests/SimplenewsAdministrationTest.php
@@ -379,7 +379,7 @@ class SimplenewsAdministrationTest extends SimplenewsTestBase {
     $this->assertEqual(1, count($rows));
     $this->assertEqual(current($subscribers['all']), trim((string) $rows[0]->td[0]));
-    $this->assertEqual($user->label(), trim((string) $rows[0]->td[1]->span));
+    $this->assertEqual($user->label(), trim((string) $rows[0]->td[1]->a));

seen this before in other cases, I think this is a permission thing/change.

I can't reproduce the failure reported in #12.

seen this before in other cases, I think this is a permission thing/change

No doubt you are right. The question is: in what way does it change?

1. If at some time in the past it used to always be a span then it changed, and now it's always a link, then my patch is good.
2. If it randomly changes between span and a, then I agree, we need a different fix.

I can see there is still uncertainty, but I feel that my patch takes us forward. It might solve 2 problems and, if not, it gives us more information. On the other hand, the existing code failing again and again each night is telling us nothing new.

Please would you commit my patch so we can see what happens? I can follow up with more patches based on what we learn. It might take a few iterations, but we'll get this solved in the end.

Committed the change, I think this is fine.

Still some more issues but hopefully non-frequent randoms, we'll see.

Re-opening, https://www.drupal.org/node/26416/qa shows varying results. some branches passed, others did not (PHP 7.3 is mostly core issues)

I think the a/span change is something that changes in 8.7, so it is failing on 8.6. Maybe we can make the condition flexible enough to work with both a and span. See #3025597: Fix failing tests how I did that for TMGMT.

For now I changed the patch/issue test configuration back to 8.7, I usually prefer to set it to pre-release.

Many thanks @Berdir

#17 is a good fallback option, but I would prefer to pin the tests down. It seems like this should be a link, and maybe there's a bug if it's not. Here is an attempt to give extra permission just in case.

Yes, it might be related to your work in core around administer user permssions?

Darn, the 8.6 one still failed. I'll do some more investigation.

Yes, it might be related to your work in core around administer user permssions?

Yes, this one #2921114: Username formatter ignores entity access.

Tricky as patch #19 works locally on D8.6.

Randomly trying a different permission in case that works.

Hurrah! Hopefully that's the end of this tedious issue - please can this be committed?

Ooops hold on, that test was D8.7. Just queued D8.6.

Bah:-(

OK back to suggestion in #17

Yay at last, thank goodness I was bored of that issue. This should get all the tests back working, please commit.

Thanks, committed.

The status and assigned fields sometimes appear with the wrong values on my system, so let's try saving again.