Skip to content

Run feature specs prebuild with unset Webpack env#6397

Merged
sheldon-b merged 4 commits intomainfrom
aduth-feature-spec-build-env
May 23, 2022
Merged

Run feature specs prebuild with unset Webpack env#6397
sheldon-b merged 4 commits intomainfrom
aduth-feature-spec-build-env

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 23, 2022

Why: Avoid potential conflicts for...

  1. Environment variables which can affect intended build output expecting Webpack dev server (source)
  2. False negatives for Make "nothing to do" if environment variable would impact build artifact, by instead always running the build

Previously: #6392

Slack context: https://gsa-tts.slack.com/archives/C0NGESUN5/p1653077779604529

**Why**: Avoid potential conflicts for...

1. Environment variables which can affect intended build output expecting Webpack dev server
2. False negatives for Make "nothing to do" if environment variable would impact build artifact

Previously: #6392

changelog: Internal, Automated Testing, Improve developer ergonomics for running JavaScript-enabled integration specs
Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@sheldon-b
Copy link
Contributor

sheldon-b commented May 23, 2022

Only downside is that this seems to be called multiple times when running multiple spec files so doing something like SHOW_BROWSER=true bundle exec rspec spec/features/ is very slow and spammy. I wonder if there's a way to only do it once. Example:

example
sbachstein@Sheldons-MBP identity-idp % SHOW_BROWSER=true bundle exec rspec spec/features/
warning: parser/current is loading parser/ruby30, which recognizes3.0.4-compliant syntax, but you are running 3.0.3.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

Randomized with seed 15124

Password recovery via personal key
Bundling JavaScript...
yarn run v1.22.18
$ yarn run clean
$ rm -rf public/packs/*
$ webpack
assets by chunk 6.91 MiB (id hint: vendors) 10 assets
assets by chunk 675 KiB (name: document-capture)
  assets by chunk 11.5 KiB (name: verify-flow) 6 assets
  4 assets
assets by chunk 55.5 KiB (name: pw-strength) 4 assets
assets by chunk 35 KiB (name: countdown_component) 4 assets
assets by chunk 33.5 KiB (name: otp-delivery-preference) 4 assets
assets by chunk 28.4 KiB (name: form-validation)
  asset js/form-validation.js 28 KiB [emitted] (name: form-validation)
  asset js/form-validation.es.js 145 bytes [emitted] (name: form-validation)
  asset js/form-validation.en.js 144 bytes [emitted] (name: form-validation)
  asset js/form-validation.fr.js 141 bytes [emitted] (name: form-validation)
32 assets
runtime modules 65.2 KiB 130 modules
orphan modules 364 KiB [orphan] 80 modules
modules by path ./node_modules/ 3.1 MiB 564 modules
modules by path ./app/ 552 KiB
  modules by path ./app/javascript/packages/ 471 KiB 120 modules
  modules by path ./app/javascript/packs/ 67.5 KiB 25 modules
  modules by path ./app/components/ 645 bytes
    modules by path ./app/components/*.ts 370 bytes 5 modules
    modules by path ./app/components/*.js 275 bytes 3 modules
  modules by path ./app/javascript/app/ 12.5 KiB
    modules by path ./app/javascript/app/components/*.js 4.82 KiB 2 modules
    modules by path ./app/javascript/app/*.js 5.15 KiB 2 modules
    ./app/javascript/app/utils/events.js 2.5 KiB [built] [code generated]
webpack 5.66.0 compiled successfully in 3276 ms
✨  Done in 3.97s.
  resets password and reactivates profile with no personal key (FAILED - 1)
  resets password, not allowed to use personal key as 2fa
Bundling JavaScript...
yarn run v1.22.18
$ yarn run clean
$ rm -rf public/packs/*
$ webpack
assets by chunk 6.91 MiB (id hint: vendors) 10 assets
assets by chunk 675 KiB (name: document-capture)
  assets by chunk 11.5 KiB (name: verify-flow) 6 assets
  4 assets
assets by chunk 55.5 KiB (name: pw-strength) 4 assets
assets by chunk 35 KiB (name: countdown_component) 4 assets
assets by chunk 33.5 KiB (name: otp-delivery-preference) 4 assets
assets by chunk 28.4 KiB (name: form-validation)
  asset js/form-validation.js 28 KiB [emitted] (name: form-validation)
  asset js/form-validation.es.js 145 bytes [emitted] (name: form-validation)
  asset js/form-validation.en.js 144 bytes [emitted] (name: form-validation)
  asset js/form-validation.fr.js 141 bytes [emitted] (name: form-validation)
32 assets
runtime modules 65.2 KiB 130 modules
orphan modules 364 KiB [orphan] 80 modules
modules by path ./node_modules/ 3.1 MiB 564 modules
modules by path ./app/ 552 KiB
  modules by path ./app/javascript/packages/ 471 KiB 120 modules
  modules by path ./app/javascript/packs/ 67.5 KiB 25 modules
  modules by path ./app/components/ 645 bytes
    modules by path ./app/components/*.ts 370 bytes 5 modules
    modules by path ./app/components/*.js 275 bytes 3 modules
  modules by path ./app/javascript/app/ 12.5 KiB
    modules by path ./app/javascript/app/components/*.js 4.82 KiB 2 modules
    modules by path ./app/javascript/app/*.js 5.15 KiB 2 modules
    ./app/javascript/app/utils/events.js 2.5 KiB [built] [code generated]
webpack 5.66.0 compiled successfully in 3273 ms
✨  Done in 3.95s.
  resets password and reactivates profile with personal key
  account recovery alternative paths
    resets password, chooses to reverify on personal key entry page
Bundling JavaScript...
yarn run v1.22.18
$ yarn run clean
$ rm -rf public/packs/*
$ webpack
assets by chunk 6.91 MiB (id hint: vendors) 10 assets
assets by chunk 675 KiB (name: document-capture)
  assets by chunk 11.5 KiB (name: verify-flow) 6 assets
  4 assets
assets by chunk 55.5 KiB (name: pw-strength) 4 assets
assets by chunk 35 KiB (name: countdown_component) 4 assets
assets by chunk 33.5 KiB (name: otp-delivery-preference) 4 assets
assets by chunk 28.4 KiB (name: form-validation)
  asset js/form-validation.js 28 KiB [emitted] (name: form-validation)
  asset js/form-validation.es.js 145 bytes [emitted] (name: form-validation)
  asset js/form-validation.en.js 144 bytes [emitted] (name: form-validation)
  asset js/form-validation.fr.js 141 bytes [emitted] (name: form-validation)
32 assets
runtime modules 65.2 KiB 130 modules
orphan modules 364 KiB [orphan] 80 modules
modules by path ./node_modules/ 3.1 MiB 564 modules
modules by path ./app/ 552 KiB
  modules by path ./app/javascript/packages/ 471 KiB 120 modules
  modules by path ./app/javascript/packs/ 67.5 KiB 25 modules
  modules by path ./app/components/ 645 bytes
    modules by path ./app/components/*.ts 370 bytes 5 modules
    modules by path ./app/components/*.js 275 bytes 3 modules
  modules by path ./app/javascript/app/ 12.5 KiB
    modules by path ./app/javascript/app/components/*.js 4.82 KiB 2 modules
    modules by path ./app/javascript/app/*.js 5.15 KiB 2 modules
    ./app/javascript/app/utils/events.js 2.5 KiB [built] [code generated]
webpack 5.66.0 compiled successfully in 3245 ms

@zachmargolis
Copy link
Contributor

zachmargolis commented May 23, 2022

Only downside is that this seems to be called multiple times when running multiple spec files so doing something

Maybe we switch to before(:suite) because before(:all) is before each group?

@aduth
Copy link
Contributor Author

aduth commented May 23, 2022

Hm, yeah, the hope was that it would only run once for the entire test run. If :suite is what we need to get that effect, makes sense to switch.

Edit: Updated in 25a4db2.

So that it's run only once for all tests

Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>
Co-Authored-By: Sheldon Bachstein <bachsteinsk@gmail.com>

if !ENV['CI']
config.before(:all, js: true) do
config.before(:suite, js: true) do
Copy link
Contributor Author

@aduth aduth May 23, 2022

Choose a reason for hiding this comment

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

Just noticed this warning when run locally:

WARNING: :suite hooks do not support metadata since they apply to the suite as a whole rather than any individual example or example group that has metadata. The metadata you have provided ([{:js=>true}]) will be ignored.

Unfortunate, since it's rather wasteful to run the Webpack build if not concerned with JavaScript-enabled feature specs 🤔 I wonder if there's another way to apply it at per-spec/group level, but only run once. Global var? 😬

Suggested change
config.before(:suite, js: true) do
config.before(js: true) do
next if defined?($ran_webpack_build)
$ran_webpack_build = true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some testing, the above suggestion does seem to work as expected. Obviously globals are gross, so happy to apply any alternative ideas, but otherwise will move forward with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this seems fine for now, better to fix the issue

@sheldon-b sheldon-b merged commit 85c1a48 into main May 23, 2022
@sheldon-b sheldon-b deleted the aduth-feature-spec-build-env branch May 23, 2022 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants