Cherry Pick: Fix the dependency of DevModeInitializer on ServletRegistration (#7709) #7744
Conversation
* Fix the dependency of DevModeInitializer on ServletRegistration * Use VaadinServlet registration if present in DevModeInitializer (cherry picked from commit d693b47)
mehdi-vaadin
added this to External Reviews
in OLD Vaadin Flow ongoing work (Vaadin 10+)
via automation
mehdi-vaadin
moved this from External Reviews
to Iteration Reviews
in OLD Vaadin Flow ongoing work (Vaadin 10+)
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status:complete! all discussions resolved, 1 of 1 LGTMs obtained
mehdi-vaadin
moved this from Changes Requested
to Iteration Reviews
in OLD Vaadin Flow ongoing work (Vaadin 10+)
There was a problem hiding this comment.
Reviewed 1 of 3 files at r2.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained, and 1 stale (waiting on @denis-anisimov and @mehdi-vaadin)
flow-server/src/test/java/com/vaadin/flow/server/startup/DevModeInitializerTest.java, line 217 at r2 (raw file):
System.setProperty( Constants.VAADIN_PREFIX + FrontendUtils.PROJECT_BASEDIR, initParams.get(FrontendUtils.PROJECT_BASEDIR));
Is there any way to avoid using system property ?
As I mentioned : it's global which mean it's not tread safe.
Other test which sets this property may break this test. Or this test may break other test which reads this property.
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained, and 1 stale (waiting on @denis-anisimov)
flow-server/src/test/java/com/vaadin/flow/server/startup/DevModeInitializerTest.java, line 217 at r2 (raw file):
Previously, denis-anisimov (Denis) wrote…
System.setProperty( Constants.VAADIN_PREFIX + FrontendUtils.PROJECT_BASEDIR, initParams.get(FrontendUtils.PROJECT_BASEDIR));Is there any way to avoid using system property ?
As I mentioned : it's global which mean it's not tread safe.
Other test which sets this property may break this test. Or this test may break other test which reads this property.
I used ServletContext parameter as you suggested on the other PR.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:
Fix the dependency of DevModeInitializer on ServletRegistration
Use VaadinServlet registration if present in DevModeInitializer
(cherry picked from commit d693b47)
This change is