Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests - do not rely on hardcoded/expected names #1245

Merged
merged 31 commits into from
Jun 11, 2020
Merged

Conversation

ibelar
Copy link
Contributor

@ibelar ibelar commented Jun 5, 2020

merge after: #1251

Set URL used in some unit testing file manually so they become independent of class naming.

Update unit test for certain test in order to make their test URL independent of the class name structure convention that usually applies with Callbacks uses in Modal, Loader, jsSSE, Form, or jsReload.

  • Move these test in separate directory: _unit-test;
  • Manually set their callback URL;
  • Move some test into Behat (Selenium) : tabs, card, table filter.

@ibelar ibelar marked this pull request as draft June 5, 2020 13:31
@codecov
Copy link

codecov bot commented Jun 5, 2020

Codecov Report

Merging #1245 into develop will decrease coverage by 1.64%.
The diff coverage is 88.88%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1245      +/-   ##
=============================================
- Coverage      72.27%   70.62%   -1.65%     
- Complexity      2555     2560       +5     
=============================================
  Files            130      130              
  Lines           6315     6319       +4     
=============================================
- Hits            4564     4463     -101     
- Misses          1751     1856     +105     
Impacted Files Coverage Δ Complexity Δ
src/FormField/TreeItemSelector.php 69.69% <ø> (ø) 11.00 <0.00> (ø)
src/ItemsPerPageSelector.php 72.41% <ø> (ø) 9.00 <0.00> (ø)
src/Popup.php 78.37% <ø> (ø) 36.00 <0.00> (ø)
src/VirtualPage.php 70.00% <ø> (ø) 21.00 <0.00> (ø)
src/Wizard.php 81.15% <50.00%> (-4.14%) 23.00 <0.00> (+1.00) ⬇️
src/Console.php 75.17% <100.00%> (+0.17%) 45.00 <0.00> (+1.00)
src/Grid.php 64.73% <100.00%> (ø) 77.00 <0.00> (+1.00)
src/Loader.php 96.00% <100.00%> (+0.16%) 11.00 <0.00> (+1.00)
src/Modal.php 88.29% <100.00%> (-3.10%) 37.00 <0.00> (+1.00) ⬇️
src/ActionExecutor/jsUserAction.php 24.13% <0.00%> (-51.73%) 16.00% <0.00%> (ø%)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26def20...3899e7a. Read the comment docs.

src/Wizard.php Outdated Show resolved Hide resolved
demos/_unit-test/sse.php Outdated Show resolved Hide resolved
demos/_unit-test/sse.php Outdated Show resolved Hide resolved
@ibelar ibelar marked this pull request as ready for review June 5, 2020 19:43
@ibelar ibelar marked this pull request as draft June 5, 2020 19:47
@ibelar
Copy link
Contributor Author

ibelar commented Jun 5, 2020

Revert this to draft - I forgot to add behat test for basic action executor

@ibelar ibelar marked this pull request as ready for review June 5, 2020 21:22
@mvorisek mvorisek changed the title fix - Unit testing url Fix tests - do not rely on hardcoded/expected names Jun 5, 2020
Copy link
Member

@mvorisek mvorisek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yes fully compatible with differently generated names:

image

clone this PR, run composer install and edit vendor/core/src/TrackableTrait.php like in the screen to retest, ideally fot Behat as well

PHPUnit 9.2.1 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.5
Configuration: dequ\ui\phpunit.xml.dist

...............................................................  63 / 317 ( 19%)
............................................................... 126 / 317 ( 39%)
............................................................... 189 / 317 ( 59%)
...............F.................................F.F........FFF 252 / 317 ( 79%)
...FF.......................................................... 315 / 317 ( 99%)
..                                                              317 / 317 (100%)

Time: 01:34.739, Memory: 10.00 MB

There were 8 failures:

1) atk4\ui\tests\GridTest::test2
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td>{$email}</td><td><a href="#" title="Delete {$email}?" class="delete"><i class="ui red trash icon"></i>Delete</a></td>'
+'<td>{$email}</td><td><a href="#" title="Delete {$email}?" class="deletex"><i class="ui red trash icon"></i>Delete</a></td>'

dequ\ui\tests\GridTest.php:63
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

2) atk4\ui\tests\TableColumnColorRatingTest::testValueGreaterThanMax
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td>{$name}</td><td>{$ref}</td><td style="{$_colorrating_color_rating}">{$rating}</td>'
+'<td>{$name}</td><td>{$ref}</td><td style="{$_colorratingx_color_rating}">{$rating}</td>'

dequ\ui\tests\TableColumnColorRatingTest.php:59
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

3) atk4\ui\tests\TableColumnColorRatingTest::testValueLowerThanMin
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td>{$name}</td><td>{$ref}</td><td style="{$_colorrating_color_rating}">{$rating}</td>'
+'<td>{$name}</td><td>{$ref}</td><td style="{$_colorratingx_color_rating}">{$rating}</td>'

dequ\ui\tests\TableColumnColorRatingTest.php:109
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

4) atk4\ui\tests\TableColumnLinkTest::testTDLast
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td>{$name}</td><td>{$ref}</td><td class="{$_money_class} right aligned single line">{$salary}</td>'
+'<td>{$name}</td><td>{$ref}</td><td class="{$_moneyx_class} right aligned single line">{$salary}</td>'

dequ\ui\tests\TableColumnLinkTest.php:56
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17
+'<td class="{$_moneyx_class} right aligned single line">{$name}</td><td>{$ref}</td><td class="{$_moneyx_2_class} right aligned single line"><b>{$salary}</b></td>'

dequ\ui\tests\TableColumnLinkTest.php:89
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

7) atk4\ui\tests\TableColumnLinkTest::testLink1
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td><a href="{$c_link}">{$name}</a></td><td>{$ref}</td>'
+'<td><a href="{$c_linkx}">{$name}</a></td><td>{$ref}</td>'

dequ\ui\tests\TableColumnLinkTest.php:149
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

8) atk4\ui\tests\TableColumnLinkTest::testLink1a
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<td><a href="{$c_link}">{$name}</a></td><td>{$ref}</td>'
+'<td><a href="{$c_linkx}">{$name}</a></td><td>{$ref}</td>'

dequ\ui\tests\TableColumnLinkTest.php:164
dequ\ui\vendor\atk4\core\src\AtkPhpunit\TestCase.php:17

FAILURES!
Tests: 317, Assertions: 1029, Failures: 8.

@ibelar
Copy link
Contributor Author

ibelar commented Jun 7, 2020

Actually the generator will change soon 🚀

All right. Done!

@mvorisek mvorisek force-pushed the fix/unit-test-url branch 2 times, most recently from 69cc7aa to 4f8d19a Compare June 10, 2020 22:09
*
* @param $arg1
*/
public function iWait($arg1)
public function iSleep($arg1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibelar better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in Behat feature step, it should represent a user action interacting with the browser. A user would rather 'wait' then 'sleep' before doing another action.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept all other (non-dummy) waits. But as dummy wait() (without test function) is a sleep (correct me if I am wrong), then sleep is much more self explaining.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have to wait until dom element finishes transitioning, but you only know visually, but as a user, you are still waiting not falling asleep. ;)

Copy link
Member

@mvorisek mvorisek Jun 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

;) then, now it is much clearer if a user sleep for a fixed TIME or wait (ie. with fixed TIMEOUT, but TIME can be shorter) for something.

I retested this PR and all fixed names are solved. Great work! Can we merge or do you have more ideas to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, if you sleep in front of your computer, you might have a very boring app in front of you. :-) Go ahead, we can merge.

Copy link
Member

@mvorisek mvorisek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweetest approval, @ibelar!

🚀 once complete

@@ -22,7 +24,7 @@ public function __construct()
/** @var null Temporary store button id when press. Use in js callback test. */
protected $buttonId;

public function getSession($name = null): Behat\Mink\Session
public function getSession($name = null): \Behat\Mink\Session
Copy link
Member

@mvorisek mvorisek Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly for this I added NS - for consistent CS

@mvorisek mvorisek merged commit aa5e79b into develop Jun 11, 2020
@mvorisek mvorisek deleted the fix/unit-test-url branch June 11, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants