Problem/Motivation

Entity browser is currently lacking test coverage. This is specially noticeable in the area of the Entity reference field widget. Probably the biggest reason for this is lack of support for Javascript testing. Recently we got JavascriptTestBase in core and it seems that we could use it to improve the situation.

Proposed resolution

Create Javascript test which will test Entity reference field widget and related entity browser.

Later we will add more tests in this area, which will cover more complex situations (nested Inline entity forms, nested Paragraphs, ...).

CommentFileSizeAuthor
#55 2765789_55.patch946 bytesmtodor
#50 interdiff.txt3.79 KBslashrsm
#50 2765789_50.patch37.04 KBslashrsm
#49 interdiff.txt3 KBslashrsm
#49 2765789_49.patch36.68 KBslashrsm
#46 interdiff.txt734 bytesslashrsm
#46 2765789_46.patch36.64 KBslashrsm
#43 interdiff.txt6.18 KBslashrsm
#43 2765789_43.patch36.14 KBslashrsm
#40 interdiff-2765789-34-40.txt650 bytesmtodor
#40 2765789-40.patch36.05 KBmtodor
#37 interdiff-2765789-34-37.txt617 bytesmtodor
#37 2765789-37.patch36.07 KBmtodor
#34 interdiff-2765789-33-34.txt18.7 KBmtodor
#34 2765789-34.patch36.04 KBmtodor
#33 interdiff-2765789-31-33.txt989 bytesmtodor
#33 2765789-33.patch38.45 KBmtodor
#31 interdiff-2765789-28-31.txt1.5 KBmtodor
#31 2765789-31.patch38.49 KBmtodor
#28 interdiff.txt2.38 KBslashrsm
#28 2765789_28.patch38.53 KBslashrsm
#26 interdiff.txt844 bytesslashrsm
#26 2765789_26.patch38.53 KBslashrsm
#23 interdiff.txt730 bytesslashrsm
#23 2765789_23.patch38.94 KBslashrsm
#20 interdiff.txt1.65 KBslashrsm
#20 2765789_20.patch38.92 KBslashrsm
#15 interdiff-2765789-7-15.txt547 byteschr.fritsch
#15 test_create_basic-2765789-15.patch38.52 KBchr.fritsch
#7 test_create_basic-2765789-7.patch38.47 KBthenchev
#6 test_create_basic-2765789-6.patch17.81 KBthenchev
#6 interdiff-2765789-6.txt12.16 KBthenchev
#3 test_create_basic-2765789-3.patch23.65 KBthenchev

Comments

slashrsm created an issue. See original summary.

slashrsm’s picture

slashrsm commented
Issue tags: +RC blocker
thenchev’s picture

thenchev commented
Assigned: Unassigned » thenchev
Status: Active » Needs review
StatusFileSize
new test_create_basic-2765789-3.patch23.65 KB

Work in progress. There is still cleanup to do and removing some duplicate code and cs. Currently I have an issue when uploading files with the upload widget and selection on the drop down.

Continuing with the remaining tests and will come back to it later.

Status: Needs review » Needs work

The last submitted patch, 3: test_create_basic-2765789-3.patch, failed testing.

The last submitted patch, 3: test_create_basic-2765789-3.patch, failed testing.

thenchev’s picture

thenchev commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2765789-6.txt12.16 KB
new test_create_basic-2765789-6.patch17.81 KB

Hope this makes it green. some dependency problems and some old code in views yml file so i updated the view.

thenchev’s picture

thenchev commented
StatusFileSize
new test_create_basic-2765789-7.patch38.47 KB

expanded tests, commented out what is currently not working.

Status: Needs review » Needs work

The last submitted patch, 7: test_create_basic-2765789-7.patch, failed testing.

The last submitted patch, 7: test_create_basic-2765789-7.patch, failed testing.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs work » Needs review

Lets re-run. Works on my local machine, maybe an unrelated error. But is it correct the the patch runs against Drupal 8.2? It should be against 8.1.7 or 8.1.x-dev

Status: Needs review » Needs work

The last submitted patch, 7: test_create_basic-2765789-7.patch, failed testing.

The last submitted patch, 7: test_create_basic-2765789-7.patch, failed testing.

The last submitted patch, 7: test_create_basic-2765789-7.patch, failed testing.

AsadKamil’s picture

AsadKamil commented

Latest patch applied cleanly to 8.x branch
Thanks

root@asad-Vostro-3550:/var/www/html/git/entity_browser# git apply -v test_create_basic-2765789-7.patch 
Checking patch config/schema/entity_browser.schema.yml...
Checking patch tests/modules/entity_browser_test/config/install/entity_browser.browser.test_entity_browser_file.yml...
Checking patch tests/modules/entity_browser_test/config/install/views.view.files_entity_browser.yml...
Checking patch tests/modules/entity_browser_test/config/install/views.view.test_selection_display_view.yml...
Checking patch tests/modules/entity_browser_test/entity_browser_test.info.yml...
Checking patch tests/src/FunctionalJavascript/EntityBrowserTest.php...
Checking patch tests/src/FunctionalJavascript/PluginsTest.php...
Applied patch config/schema/entity_browser.schema.yml cleanly.
Applied patch tests/modules/entity_browser_test/config/install/entity_browser.browser.test_entity_browser_file.yml cleanly.
Applied patch tests/modules/entity_browser_test/config/install/views.view.files_entity_browser.yml cleanly.
Applied patch tests/modules/entity_browser_test/config/install/views.view.test_selection_display_view.yml cleanly.
Applied patch tests/modules/entity_browser_test/entity_browser_test.info.yml cleanly.
Applied patch tests/src/FunctionalJavascript/EntityBrowserTest.php cleanly.
Applied patch tests/src/FunctionalJavascript/PluginsTest.php cleanly.
root@asad-Vostro-3550:/var/www/html/git/entity_browser#
chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new test_create_basic-2765789-15.patch38.52 KB
new interdiff-2765789-7-15.txt547 bytes

I fixed on of the PluginTests

Status: Needs review » Needs work

The last submitted patch, 15: test_create_basic-2765789-15.patch, failed testing.

The last submitted patch, 15: test_create_basic-2765789-15.patch, failed testing.

chr.fritsch’s picture

chr.fritsch
he/him
Primary language German
Location 🇩🇪🇪🇺🌍
commented

Ok, i fixed one of four failing tests. Any ideas why the others are failing? Locally everything works fine. I don't get it.

thenchev’s picture

thenchev commented

Have also problems with test failing sometimes and then randomly start working. Not exactly sure. Uploaded the patch to see how testbot behaves. Will look into it later but having problems getting consistent behavior...

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_20.patch38.92 KB
new interdiff.txt1.65 KB

This is green locally. Let's see what tesbot says about it.

Status: Needs review » Needs work

The last submitted patch, 20: 2765789_20.patch, failed testing.

The last submitted patch, 20: 2765789_20.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_23.patch38.94 KB
new interdiff.txt730 bytes

Let's try this.

Status: Needs review » Needs work

The last submitted patch, 23: 2765789_23.patch, failed testing.

The last submitted patch, 23: 2765789_23.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_26.patch38.53 KB
new interdiff.txt844 bytes

Status: Needs review » Needs work

The last submitted patch, 26: 2765789_26.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_28.patch38.53 KB
new interdiff.txt2.38 KB

Status: Needs review » Needs work

The last submitted patch, 28: 2765789_28.patch, failed testing.

The last submitted patch, 28: 2765789_28.patch, failed testing.

mtodor’s picture

mtodor commented
Status: Needs work » Needs review
StatusFileSize
new 2765789-31.patch38.49 KB
new interdiff-2765789-28-31.txt1.5 KB

Just to see, what Testbot will say.

Status: Needs review » Needs work

The last submitted patch, 31: 2765789-31.patch, failed testing.

mtodor’s picture

mtodor commented
Status: Needs work » Needs review
StatusFileSize
new 2765789-33.patch38.45 KB
new interdiff-2765789-31-33.txt989 bytes

Adjusted failing test.

Btw. This is not finished yet. I'm just testing Testbot.

  1. We need to do some additional refactoring, making one base class with helper methods, setUp, etc. Because we have a lot duplicated code there.
mtodor’s picture

mtodor commented
StatusFileSize
new 2765789-34.patch36.04 KB
new interdiff-2765789-33-34.txt18.7 KB

Small refactoring.

@slashrsm: We should continue with this ticket. What is next step?
I would like to see this ticket finished soon, so that we have base for making tests in other tickets too.
A lot of things are fixed these days, but we didn't cover anything with tests.

Status: Needs review » Needs work

The last submitted patch, 34: 2765789-34.patch, failed testing.

The last submitted patch, 34: 2765789-34.patch, failed testing.

mtodor’s picture

mtodor commented
Status: Needs work » Needs review
StatusFileSize
new 2765789-37.patch36.07 KB
new interdiff-2765789-34-37.txt617 bytes

Added @group in Base class.

Status: Needs review » Needs work

The last submitted patch, 37: 2765789-37.patch, failed testing.

The last submitted patch, 37: 2765789-37.patch, failed testing.

mtodor’s picture

mtodor commented
Status: Needs work » Needs review
StatusFileSize
new 2765789-40.patch36.05 KB
new interdiff-2765789-34-40.txt650 bytes

Added @group in Base class.
Change Base class to abstract.

Status: Needs review » Needs work

The last submitted patch, 40: 2765789-40.patch, failed testing.

The last submitted patch, 40: 2765789-40.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_43.patch36.14 KB
new interdiff.txt6.18 KB

Few coding standard fixes and added missing schema.

Status: Needs review » Needs work

The last submitted patch, 43: 2765789_43.patch, failed testing.

The last submitted patch, 43: 2765789_43.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_46.patch36.64 KB
new interdiff.txt734 bytes

Status: Needs review » Needs work

The last submitted patch, 46: 2765789_46.patch, failed testing.

The last submitted patch, 46: 2765789_46.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Needs review
StatusFileSize
new 2765789_49.patch36.68 KB
new interdiff.txt3 KB
slashrsm’s picture

slashrsm commented
Related issues: +#2771547: In Browser and FunctionalJavascript tests SIMPLETEST_USER_AGENT cookie needs to be set every 5 seconds
StatusFileSize
new 2765789_50.patch37.04 KB
new interdiff.txt3.79 KB

Uncomment few tests/asserts.

Some problems originate in #2771547: In Browser and FunctionalJavascript tests SIMPLETEST_USER_AGENT cookie needs to be set every 5 seconds. We should be able to uncomment more stuff when that lands.

Status: Needs review » Needs work

The last submitted patch, 50: 2765789_50.patch, failed testing.

slashrsm’s picture

slashrsm commented
Status: Needs work » Reviewed & tested by the community

I think that we can commit this patch. It is not perfect, but it is waaay better than what we currently have.

I will wait a bit to hear what other contributors to this issue say.

  • slashrsm committed 31f34ff on 8.x-1.x authored by Denchev 
    Issue #2765789 by slashrsm, mtodor, Denchev, chr.fritsch: Create basic...
slashrsm’s picture

slashrsm commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests, -RC blocker

Yay!

mtodor’s picture

mtodor commented
Status: Fixed » Needs review
StatusFileSize
new 2765789_55.patch946 bytes

I see this is already merged. I have attached patch (based on last commit), that should fix test fail (flip-flop) at:

Previous test did check for "llama.jpg" inside entity browser iframe, not in article and I guess that's wrong.
And test would fail on fast execution, that's why it's flip-flopping on PHP 7.

slashrsm’s picture

slashrsm commented
Status: Needs review » Fixed

Committed.

  • slashrsm committed 219dc02 on 8.x-1.x authored by mtodor 
    Issue #2765789 by mtodor: Make functional test more robust.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.