Make alert widget not access all flash data (Fixes #250)

[tobias-dietz]

Q	A
Is bugfix?	yes
New feature?	no
Breaks BC?	no
Tests pass?	yes
Fixed issues	#250

[yii-bot]

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests.

Could you add these please?

Thanks!

P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests

This is an automated comment, triggered by adding the label pr:request for unit tests.

[tobias-dietz]

Found the missing spaces and added them 😄 I've also added a unit test. The tests may seem verbose but it felt nessecary since the alert widget has a nested foreach loop and can process strings as well as arrays for the differnt message types. If it's too verbose, I would suggest to remove all tests except for the last three ones.

The last test tests the fix for this bug and should fail with the current alert widget implementation in master but not with the fixed implementation.

[bizley]

Wow, impressive, not everyday we can see so robust testing. One tiny thing - it should be PHPUnit tests strictly, not Codeception so please change the extended test case class and implementation details there (but it should be pretty straight forward if I'm not mistaken).
Also please add CHANGELOG line. Thank you!

[tobias-dietz]

Thank you for your nice feedback 😄 I've pushed a new commit, the unit test looks now the same as the existing ones for the models (tests\unit\models\LoginFormTest etc.).
However, I have the strong feeling that I did not understand exactly what you meant because those expect() are looking like they only wrap Codeception methods.

I tried to extend PHPUnit\Framework\TestCase but then I receive a lot of errors like [yii\base\ErrorException] Attempt to read property "session" on null (which wasn't that surprising to be honest).

Then I found https://github.com/yiisoft/yii2/blob/master/tests/TestCase.php which I think is what you meant? But it seems that this class is only part of yiisoft/yii2 and not directly usable in yiisoft/yii2-app-basic (at least, PhpStorm doesn't offer that class on auto complete)? If I'm wrong (what is not that unlikely), it would be nice if you could point on additional resources or an example. I followed the TESTING section of the README file in yiisoft/yii2-app-basic so far.

Regarding the CHANGELOG: I couldn't find a CHANGELOG in yiisoft/yii2-app-basic.

[bizley]

Oh, right, no changelog here, weird... Anyway, it looks like I went full-auto here and treated it just like Yii 2 core while it's not the case and the few tests here are indeed Codeception ones - could you please revert the changes I've told you to do and bring back the ones you made before? I'm very very sorry for that extra work 😭

[tobias-dietz]

No problem 😄 The commit was broken either way (forgot to clean up my experiments with PHPUnit TestCase). Please have a look again. Basically, they are my original tests, only now with the Codeception expect() functions (instead of accessing $this->tester) that are also used in the other tests in yii2-app-basic (to maintain visual consistency with the other tests).

[machour]

Nice PR, thank you for your contribution.

[samdark]

Thanks!

[samdark]

Would you please apply the same to advanced app template?