Proxy webpack dev server in test environment
What does this MR do and why?
This MR enables the test environment to proxy webpack dev server requests. Issues were discovered while working on this MR.
This fixes issues with locally running rspecs that call resetServiceWorkersPublicPath
. Rather than try to unravel why we need to call this in the first place, this MR opts to just enable proxying the dev server for consistency with the dev environment and to minimize the effect of this change.
How to set up and validate locally
- Apply the changes from this MR (if it's not merged to
master
yet) ontop of this branch - Run
bundle exec spring rspec spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb
- Previously this will fail and the console would show a number of chunks not loading
Internal Slack discussion
I found a lead... The page that fails tries to pull assets from http://127.0.0.1:42293/ (the rspec host) while the page that succeeds pulls from the webpack dev server.
2 files
markrian 1 hour ago
I guess that also explains the CSP errors :thinking_face: Or rather, the CSP errors are why the chunks aren't loading? (edited)
pslaughter 1 hour ago
It looks like this fixes things... I'm not really sure why this is here...
diff --git a/app/assets/javascripts/lib/utils/webpack.js b/app/assets/javascripts/lib/utils/webpack.js
index a88f1bd82fc..af3f7705fd8 100644
--- a/app/assets/javascripts/lib/utils/webpack.js
+++ b/app/assets/javascripts/lib/utils/webpack.js
@@ -10,5 +10,5 @@ export function resetServiceWorkersPublicPath() {
// see: https://webpack.js.org/guides/public-path/
const relativeRootPath = (gon && gon.relative_url_root) || '';
const webpackAssetPath = joinPaths(relativeRootPath, '/assets/webpack/');
- __webpack_public_path__ = webpackAssetPath; // eslint-disable-line babel/camelcase
+ // __webpack_public_path__ = webpackAssetPath; // eslint-disable-line babel/camelcase
}
markrian 1 hour ago
That's a fix/hack for web workers... Or CDNs... Something like that :sweat_smile:
pslaughter 1 hour ago
Yeah, I think it's so that web workers are served from the same host?
markrian 1 hour ago
But that's totally unrelated to the MR we were looking at, right? How have those tests ever passed?
pslaughter 1 hour ago
It looks like it's just a local failure??
pslaughter 1 hour ago
If the webpack assets are served from the main host + the webpack host, then it'll pass. If they aren't served from the main host (like in rspec) it fails. Maybe something changed there? Maybe they used to be served from the rspec host and aren't anymore?
pslaughter 1 hour ago
It looks like the webpack public host is only ever in a different domain if we're running tests :thinking_face:https://gitlab.com/gitlab-org/gitlab/-/commit/7b99ed566de9004ca73e6bc16287daace5c0d883.
pslaughter 1 hour ago
Maybe we should proxy webpack output in 'test' environments (it looks like that's what dev and prod do) :shrug:
markrian 1 hour ago
So that logic, interestingly, was made to improve webpack-dev-server compatibility with non-localhost setups !10604
pslaughter 24 minutes ago
So this also fixed the test issues for me:
diff --git a/app/helpers/webpack_helper.rb b/app/helpers/webpack_helper.rb
index 64900714327..ba3c232bec4 100644
--- a/app/helpers/webpack_helper.rb
+++ b/app/helpers/webpack_helper.rb
@@ -83,16 +83,8 @@ def webpack_entrypoint_paths(source, extension: nil, exclude_duplicates: true)
end
def webpack_public_host
- # We do not proxy the webpack output in the 'test' environment,
- # so we must reference the webpack dev server directly.
- if Rails.env.test? && Gitlab.config.webpack.dev_server.enabled
- host = Gitlab.config.webpack.dev_server.host
- port = Gitlab.config.webpack.dev_server.port
- protocol = Gitlab.config.webpack.dev_server.https ? 'https' : 'http'
- "#{protocol}://#{host}:#{port}"
- else
- ActionController::Base.asset_host.try(:chomp, '/')
- end
+ # We proxy webpack output in 'test' and 'dev' environment, so we can just use asset_host
+ ActionController::Base.asset_host.try(:chomp, '/')
end
def webpack_public_path
diff --git a/config/initializers/static_files.rb b/config/initializers/static_files.rb
index 2879d48387d..69409d2c59c 100644
--- a/config/initializers/static_files.rb
+++ b/config/initializers/static_files.rb
@@ -21,7 +21,7 @@
# If webpack-dev-server is configured, proxy webpack's public directory
# instead of looking for static assets
- if Gitlab.config.webpack.dev_server.enabled && Rails.env.development?
+ if Gitlab.config.webpack.dev_server.enabled && (Rails.env.development? || Rails.env.test?)
app.config.middleware.insert_before(
Gitlab::Middleware::Static,
Gitlab::Webpack::DevServerMiddleware,
I'm going to spin up an MR for this
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Paul Slaughter