Configure to run IE11 tests in production mode #4306

pekam · 2018-06-19T12:22:22Z

Trigger production mode and exclude IgnoreIE11 tests when running in BrowserStack
Fix es5 frontend path by removing the configuration that sets it the same as for es6
Move tests and required resources that depend on the above config from test-root-context to test-dev-mode
plus a couple of minor test improvements

This change is

This reverts commit 435dc81.

This reverts commit 1d40a78.

This reverts commit 6deceaa.

denis-anisimov · 2018-06-19T12:47:47Z

Reviewed 1 of 20 files at r1.
Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

flow-tests/test-dev-mode/src/main/webapp/frontend/com/vaadin/flow/uitest/vaadin-flow-bundle.html, line 1 at r1 (raw file):

<!-- Empty File -->

This and the change below will have an effect on Tomcat tests which are executed nightly so failures won't be detected during the validation.
Please make sure that Tomcat tests are working with those changes.

Comments from Reviewable

pekam · 2018-06-19T13:15:56Z

Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

flow-tests/test-dev-mode/src/main/webapp/frontend/com/vaadin/flow/uitest/vaadin-flow-bundle.html, line 1 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

This and the change below will have an effect on Tomcat tests which are executed nightly so failures won't be detected during the validation.
Please make sure that Tomcat tests are working with those changes.

I ran mvn clean install -Ptomcat in servlet-containers and got two errors for Tomcat 9:

[ERROR]   RequestParametersHistoryIT>ChromeBrowserTest.setup:62->ChromeBrowserTest.createHeadlessChromeDriver:76 ▒ SessionNotCreated
[ERROR]   RequestParametersHistoryIT>ChromeBrowserTest.setup:62->ChromeBrowserTest.createHeadlessChromeDriver:76 ▒ SessionNotCreated

Seems un-related to this, but I'll run it again.

Comments from Reviewable

pekam · 2018-06-19T13:21:13Z

Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

flow-tests/test-dev-mode/src/main/webapp/frontend/com/vaadin/flow/uitest/vaadin-flow-bundle.html, line 1 at r1 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

I ran mvn clean install -Ptomcat in servlet-containers and got two errors for Tomcat 9:
[ERROR]   RequestParametersHistoryIT>ChromeBrowserTest.setup:62->ChromeBrowserTest.createHeadlessChromeDriver:76 ▒ SessionNotCreated
[ERROR]   RequestParametersHistoryIT>ChromeBrowserTest.setup:62->ChromeBrowserTest.createHeadlessChromeDriver:76 ▒ SessionNotCreated
Seems un-related to this, but I'll run it again.

On the second try all the tests passed.

Comments from Reviewable

denis-anisimov · 2018-06-19T14:25:37Z

Review status: all discussions resolved, 0 of 1 LGTMs obtained

flow-tests/test-dev-mode/src/main/webapp/frontend/com/vaadin/flow/uitest/vaadin-flow-bundle.html, line 1 at r1 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

On the second try all the tests passed.

Good, thanks.

Comments from Reviewable

denis-anisimov · 2018-06-20T06:35:33Z

Reviewed 19 of 20 files at r1.
Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):
There are a number of UI classes that have been moved out of the root-context module.
I would like to understand reasons for each of them.
Is it so that they can't be used in production at all even with modifications?
At the first glance I don't see any reasons why they are incompatible with the production mode.

Or they have tests that are executed in production mode which is not a production mode?
There are some tests like that.

BUT.
It's about tests, not about UIs.
There are tests that uses the same UI but they are executed without any production mode. And the same IT test class has tests in production mode.
So instead of moving the UI classes out of the root-context module into dev-mode module they should be moved into common module and IT test classes splitted so that the part which doesn't relate to production mode stays in the root-context module and the part which is about semi-production mode goes to the dev-mode.

flow-tests/test-root-context/pom.xml, line 83 at r1 (raw file):

<name>vaadin.frontend.url.es5</name>

This is not needed because you have moved everything into frontend folder in other module.
But may be this has been done intentionally? (I don't know).

Comments from Reviewable

pekam · 2018-06-20T08:22:10Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):
The tests that are moved use the openProduction() and/or openForEs6Url() which don't work with the production mode.

Or they have tests that are executed in production mode which is not a production mode?

Yes.

There are tests that uses the same UI but they are executed without any production mode. And the same IT test class has tests in production mode.

I don't follow. You mean that same view is used in both test-dev-mode and test-root-context? Where are tests like this?

flow-tests/test-root-context/pom.xml, line 83 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

<name>vaadin.frontend.url.es5</name>
This is not needed because you have moved everything into frontend folder in other module.
But may be this has been done intentionally? (I don't know).

Previously, when there was no "true" production mode, this config was used to serve the same static resources for es5 that we have for es6. Otherwise it would have tried to look for non-existing transpiled resources in some es5 folder.

Now with transpilation enabled, if we want the transpiled resources to be served to IE11, we need to use the default directory for es5 which contains the transpiled resources.

Comments from Reviewable

denis-anisimov · 2018-06-20T09:03:32Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, pekam (Pekka Maanpää) wrote…

The tests that are moved use the openProduction() and/or openForEs6Url() which don't work with the production mode.

Or they have tests that are executed in production mode which is not a production mode?

Yes.

There are tests that uses the same UI but they are executed without any production mode. And the same IT test class has tests in production mode.

I don't follow. You mean that same view is used in both test-dev-mode and test-root-context? Where are tests like this?

I've seen such tests.
All the tests should be checked for this.
Let me find at least one....

ExportedJSFunctionIT.
It has methods:

versionInfoAvailableInDevelopmentMopde (name could be corrected by the way)
versionInfoNotAvailableInProductionMode

The first test uses open() which means that it doesn't require anything special to work.
It's not a dev mode actually it's rather a regular run. It will become a production mode run being running in production mode. So the name is bad (and it has extra p).
This method ( I expect this) should work in production mode as well.

The second method uses openProduction() . This "open" method changes the URL of the opened UI so that another servlet is used to run the UI for the test and this servlet has production mode enabled.
This is semi-producton mode. Which should not be used with production.

Or may be the first test relies on the production mode and cannot be run in production mode at all.
But the second test should work as is in production mode as well.

It needs to be checked. I'm not sure.
Are all the tests in ExportedJSFunctionIT failing in production mode?
If so then it should be completely moved into dev-mode.
If only some tests are failing then those tests should be extracted.

And the same for every other test.

Comments from Reviewable

denis-anisimov · 2018-06-20T09:04:28Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):

It will become a production mode run being running in production mode

Sorry .
I meant :

It will become a dev mode run being running in production mode

Comments from Reviewable

pekam · 2018-06-20T10:06:31Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

It will become a production mode run being running in production mode

Sorry .
I meant :

It will become a dev mode run being running in production mode

Yes, currently tests that use this semi-production mode are in test-dev-mode.
I don't know if it's optimal but that's how I got it to work in normal build (without -PproductionMode), since those didn't work anymore in test-root-context.

Actually I now figured out that I could have kept those in the test-root-context if I trigger the frontend es5 config which I removed only when not running in production mode.

So the configuration is currently like this:

test-dev-mode contains the "old" configuration and everything works as it used to
- views are served normally under /view/
- views are served in semi-production mode under /view-production/: only the servlet parameter is set, but no maven plugin is run
- views are served in semi-production mode with overridden frontend.url.es6 under /view-es6-url/
test-root-context has the same servlets, but it actually uses only the basic /view/ paths.
- when running in production mode with -PproductionMode it really runs the maven plugin instead of just setting one parameter, and it uses the same servlet with /view/
- other servlets still exist (/view-production/ and /view-es6-url/) but they are not used and actually they don't even work with the new configuration that expects a "true" production mode where the transpiled es5 files really exist

(I'm not sure if you needed all this explanation but I needed to write it down to clarify for myself at least)

If we want to move all the semi-production mode tests to use "real" production mode, then we would need to run transpilation and everything for every build. I don't think it's necessary since the tests just check that the resource paths are changed based on the servlet parameter. That's why I'd like to just have them in test-dev-mode.

But the modules have really bad names in this case...

Comments from Reviewable

denis-anisimov · 2018-06-20T10:26:43Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, pekam (Pekka Maanpää) wrote…

Yes, currently tests that use this semi-production mode are in test-dev-mode.
I don't know if it's optimal but that's how I got it to work in normal build (without -PproductionMode), since those didn't work anymore in test-root-context.

Actually I now figured out that I could have kept those in the test-root-context if I trigger the frontend es5 config which I removed only when not running in production mode.

So the configuration is currently like this:

test-dev-mode contains the "old" configuration and everything works as it used to

views are served normally under /view/

views are served in semi-production mode under /view-production/: only the servlet parameter is set, but no maven plugin is run

views are served in semi-production mode with overridden frontend.url.es6 under /view-es6-url/

test-root-context has the same servlets, but it actually uses only the basic /view/ paths.

when running in production mode with -PproductionMode it really runs the maven plugin instead of just setting one parameter, and it uses the same servlet with /view/

other servlets still exist (/view-production/ and /view-es6-url/) but they are not used and actually they don't even work with the new configuration that expects a "true" production mode where the transpiled es5 files really exist

(I'm not sure if you needed all this explanation but I needed to write it down to clarify for myself at least)

If we want to move all the semi-production mode tests to use "real" production mode, then we would need to run transpilation and everything for every build. I don't think it's necessary since the tests just check that the resource paths are changed based on the servlet parameter. That's why I'd like to just have them in test-dev-mode.

But the modules have really bad names in this case...

You have described the situation which happens with the current PR.

But I have some doubts....
So the questions are:

does versionInfoAvailableInDevelopmentMopde fail being running in production mode ?
does versionInfoNotAvailableInProductionMode fail being running in production mode ?

If they both are failing then it's OK to completely move UIs and tests to the dev-mode.
If not then UIs should be moved to the common, IT tests should be splitted between two modules.

Also if you are moving out all semi-production mode related tests then you may move the corresponding production mode servlet into dev-mode module out of the common module.

Comments from Reviewable

denis-anisimov · 2018-06-20T10:32:59Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

flow-tests/test-root-context/pom.xml, line 83 at r1 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

Previously, when there was no "true" production mode, this config was used to serve the same static resources for es5 that we have for es6. Otherwise it would have tried to look for non-existing transpiled resources in some es5 folder.

Now with transpilation enabled, if we want the transpiled resources to be served to IE11, we need to use the default directory for es5 which contains the transpiled resources.

Yes, but semi-production mode tests have been moved to the dev-mode.
And I don't see similar changes in the pom.xml of dev-mode to be able to handle the resources in the same way as it was in this module.

Comments from Reviewable

pekam · 2018-06-20T10:47:32Z

Review status: 2 unresolved discussions, 0 of 1 LGTMs obtained

a discussion (no related file):

does versionInfoAvailableInDevelopmentMopde fail being running in production mode ?
does versionInfoNotAvailableInProductionMode fail being running in production mode ?

The semi-production mode should be enough to verify these, as they have been using that until this point anyway.

I didn't test now but the first test should fail in production mode and the second one shouldn't. It shouldn't matter whether we use semi-production mode or true production mode here.

If not then UIs should be moved to the common, IT tests should be splitted between two modules.

Then we need to either run production build always in test-root-context, or make the semi-production mode work in there also. I think it's cleaner if we keep the test-root-context simple with just the one servlet used for both dev and production mode, and have the other servlets just in test-dev-mode.

Also if you are moving out all semi-production mode related tests then you may move the corresponding production mode servlet into dev-mode module out of the common module.

I agree with this.

flow-tests/test-root-context/pom.xml, line 83 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Yes, but semi-production mode tests have been moved to the dev-mode.
And I don't see similar changes in the pom.xml of dev-mode to be able to handle the resources in the same way as it was in this module.

We already had that config in the test-dev-mode.

Comments from Reviewable

denis-anisimov · 2018-06-20T11:05:02Z

Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

a discussion (no related file):

I didn't test now but the first test should fail in production mode and the second one shouldn't. It shouldn't matter whether we use semi-production mode or true production mode here.

I don't think so.
The production mode assumes that the servlet is running production mode AND also web resources are transpiled and are in the correct location.
The last thing is exactly which is different for this semi-production mode. And may be this is the purpose of the test:
check what's going on when the user forgets to transpile but still running the production mode in the servlet. ( I don't know anything about those tests).
So let's don't do any assumption for those tests.

Then we need to either run production build always in test-root-context, or make the semi-production mode work in there also.

Yes. I'm talking about the second option.
We don't want to run production mode all the time for root-context tests.
But why we can't use the same tests as is ?
If those tests works in "semi-production" mode AND they work in a normal production mode then I don't understand what's the problem ?
Why we can't just keep everything as is (as I said: mode UI classes in common keep the tests which are working here and move those that don't work to dev-mode)?

flow-tests/test-root-context/pom.xml, line 83 at r1 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

We already had that config in the test-dev-mode.

Ah, OK.

Comments from Reviewable

pekam · 2018-06-20T12:32:14Z

Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

a discussion (no related file):

If those tests works in "semi-production" mode AND they work in a normal production mode then I don't understand what's the problem ?

It makes things more complex and confusing.

I need to trigger specific configuration to make the semi-production mode work when not running real production mode (the configuration for frontend-es5 which I removed in this PR). So there will be both kinds of production mode in test-root-context.

Also, using the same view for both modules makes things more confusing. Usually we have the test view with the test class in the same module, but now we would need to introduce a couple of exceptions which are used in two modules.

But whatever, if you still disagree with me, I will try to make it work to get this forward.

Comments from Reviewable

denis-anisimov · 2018-06-20T13:04:35Z

Review status: 1 unresolved discussion, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, pekam (Pekka Maanpää) wrote…

If those tests works in "semi-production" mode AND they work in a normal production mode then I don't understand what's the problem ?

It makes things more complex and confusing.

I need to trigger specific configuration to make the semi-production mode work when not running real production mode (the configuration for frontend-es5 which I removed in this PR). So there will be both kinds of production mode in test-root-context.

Also, using the same view for both modules makes things more confusing. Usually we have the test view with the test class in the same module, but now we would need to introduce a couple of exceptions which are used in two modules.

But whatever, if you still disagree with me, I will try to make it work to get this forward.

If it's complicated then OK.
I'm just asking because I'm not aware of issues.
Of course it may be that I just don't see something.
And that's why I'm asking.

Don't see any issues with having shared UI classes for two modules.
They are not tests. They are additional classes which helps to test.

But if it's too hard to make tests work for root-context then it's OK just move them out.
I just wanted to be sure that it's really necessary.

Comments from Reviewable

denis-anisimov · 2018-06-20T13:05:35Z

Review status: all discussions resolved, 0 of 1 LGTMs obtained

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

If it's complicated then OK.
I'm just asking because I'm not aware of issues.
Of course it may be that I just don't see something.
And that's why I'm asking.

Don't see any issues with having shared UI classes for two modules.
They are not tests. They are additional classes which helps to test.

But if it's too hard to make tests work for root-context then it's OK just move them out.
I just wanted to be sure that it's really necessary.

We are releasing now......
Let's wait for the release and don't merge it right now.
I'm blocking this because of the release.

Comments from Reviewable

denis-anisimov · 2018-06-21T09:41:05Z

Review status: all discussions resolved, 0 of 1 LGTMs obtained, and 1 stale

Comments from Reviewable

* Trigger productionMode and IgnoreIE11 in BrowserStack * Revert productionMode activation * activate production mode only on browserstack property * Use TestBench queries in ChildOrderIT * Improve browser checking in BodyScrollIT * Try to fix CompositeIT with waitForElementPresent * Wait for shadow-root child in ChildOrderIT * Increase timeout for ChildOrderIT * Revert "Increase timeout for ChildOrderIT" This reverts commit 435dc81. * Revert "Wait for shadow-root child in ChildOrderIT" This reverts commit 1d40a78. * Revert "Use TestBench queries in ChildOrderIT" This reverts commit 6deceaa. * Merge branch 'master' into ie11-conf * Remove es5 folder config * Move ExportedJSFunction tests out of test-root-context * Move ClientSideExceptionHandling tests out of test-root-context * Move InfoIT out of test-root-context * Move frontend protocol tests out of test-root-context * Add description for flow-test-dev-mode * Merge branch 'master' into ie11-conf

pekam added 19 commits June 15, 2018 15:22

Trigger productionMode and IgnoreIE11 in BrowserStack

dc5c0fe

Revert productionMode activation

fde389f

activate production mode only on browserstack property

f9b3b40

Use TestBench queries in ChildOrderIT

6deceaa

Improve browser checking in BodyScrollIT

a68acd5

Try to fix CompositeIT with waitForElementPresent

222eff2

Wait for shadow-root child in ChildOrderIT

1d40a78

Increase timeout for ChildOrderIT

435dc81

Revert "Increase timeout for ChildOrderIT"

a059134

This reverts commit 435dc81.

Revert "Wait for shadow-root child in ChildOrderIT"

61a60a1

This reverts commit 1d40a78.

Revert "Use TestBench queries in ChildOrderIT"

d32e4d6

This reverts commit 6deceaa.

Merge branch 'master' into ie11-conf

0d4db0f

Remove es5 folder config

5779459

Move ExportedJSFunction tests out of test-root-context

f582715

Move ClientSideExceptionHandling tests out of test-root-context

9af899d

Move InfoIT out of test-root-context

850afd6

Move frontend protocol tests out of test-root-context

d925a52

Add description for flow-test-dev-mode

947b2d7

Merge branch 'master' into ie11-conf

4da67e8

pekam changed the title ~~Configure to run IE11 test in production mode~~ Configure to run IE11 tests in production mode Jun 19, 2018

pekam self-assigned this Jun 19, 2018

denis-anisimov merged commit 554be24 into master Jun 21, 2018

denis-anisimov deleted the ie11-conf branch June 21, 2018 09:41

gilberto-torrezan added this to the 1.0.1 milestone Jul 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure to run IE11 tests in production mode #4306

Configure to run IE11 tests in production mode #4306

pekam commented Jun 19, 2018 •

edited by SomeoneToIgnore

Loading

denis-anisimov commented Jun 19, 2018

pekam commented Jun 19, 2018

pekam commented Jun 19, 2018

denis-anisimov commented Jun 19, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 21, 2018

Configure to run IE11 tests in production mode #4306

Configure to run IE11 tests in production mode #4306

Conversation

pekam commented Jun 19, 2018 • edited by SomeoneToIgnore Loading

denis-anisimov commented Jun 19, 2018

pekam commented Jun 19, 2018

pekam commented Jun 19, 2018

denis-anisimov commented Jun 19, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

pekam commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 20, 2018

denis-anisimov commented Jun 21, 2018

pekam commented Jun 19, 2018 •

edited by SomeoneToIgnore

Loading