Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup testing similar to dartsass-rails #187

Closed
wants to merge 13 commits into from
Closed

Conversation

zzak
Copy link
Member

@zzak zzak commented Jan 12, 2024

This code is based on rails/dartsass-rails#49, see also hotwired/turbo-rails#553 and hotwired/stimululs-rails#136.

/cc @ksylvest so we can collaborate and filling out more test cases (I've also sent you an invite to my fork so you can push to this branch).

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Interesting that 5878a64 test for checking package.json only fails on Rails 6.1

@ksylvest
Copy link
Contributor

ksylvest commented Jan 12, 2024

@zzak thanks - you OK with pushing some fixups directly? A bit unfamiliar with Appraisals, but believe we need to regenerate?

bundle exec appraisal install

e90f88a

Also - any reason for having sqlite (assuming initially used w/ Rails::Generators::AppGenerator, but seems like skipping ActiveRecord, ActiveJob, ActionCable, and a handful of others might be prudent)?

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

@ksylvest Yeah, feel free to push whatever commits and we can squash when we're ready.

For the other installers (node, bun, esbuild, webpack), we can split them out to separate files like test/installers/test_bun_installer. Which I've started with webpack in 88bfb43, feel free to continue with that or add system tests.

We also need to do the same for cssbundling-rails, if you want to wait or can start on that too. Up to you!

Also, @flavorjones has done an excellent job with https://github.com/rails/tailwindcss-rails so there might be ideas we can pull in from there too.

Thank you!!

@ksylvest ksylvest force-pushed the test-installer branch 4 times, most recently from 2124cec to 8347367 Compare January 12, 2024 09:39
@ksylvest
Copy link
Contributor

ksylvest commented Jan 12, 2024

@zzak happy to look into cssbundling-rails. I've included tests for the three other package systems:

  1. bun: 71d6911
  2. rollup: 0adcb7b
  3. esbuild: a1e93e8

Also tweaked our test on package.json slightly for webpack (LMK if overkill). Anything else needed here?

Ensures:

1. The esbuild NPM package is installed as a dependency
2. The esbuild command is used as a build script
3. The Procfile.dev is copied / modified
Ensures:

1. The rollup NPM packages are installed as dependencies
2. The rollup command is used as a build script
3. The Procfile.dev is copied / modified
Ensures:

1. The bun config file is copied
2. The bun command is used as a build script
3. The Procfile.dev is copied / modified
@ksylvest
Copy link
Contributor

Forgot to confirm... This section of CI is an artifact of the prior life as a dartsass-rails CI.yml file right?

https://github.com/rails/jsbundling-rails/pull/187/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR48-R54

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

@ksylvest Thanks! There seem to be a few failures, and we probably want to make sure rails+ruby main don't fail the build. I will look into those and clean things up.

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Forgot to confirm... This section of CI is an artifact of the prior life as a dartsass-rails CI.yml file right?

https://github.com/rails/jsbundling-rails/pull/187/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR48-R54

Yes, that is a dart sass specific problem, I believe.

@ksylvest
Copy link
Contributor

Are you just running bundle exec appraisal rake test? I don't see any issues on local when running:

results.txt

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

I was just running this but passes locally for me too:

$  ruby -v
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin23]

$ BUNDLE_GEMFILE=gemfiles/rails_7_1_propshaft.gemfile bundle exec rake
Run options: --seed 54245

# Running:

.............

Finished in 29.328444s, 0.4433 runs/s, 1.3639 assertions/s.
13 runs, 40 assertions, 0 failures, 0 errors, 0 skips

@ksylvest
Copy link
Contributor

I might've introduced a test dependency ordering issue 🤦. I think this is missing in the webpack spec:

https://github.com/rails/jsbundling-rails/pull/187/files#diff-c54b8fa689f9a927f901d3762afa9c9523d9d3a2464d7525c3a43a03edb61f30R1

https://github.com/rails/jsbundling-rails/pull/187/files#diff-ac1bfc402d94e3e1f37ced5405775c3ad6757f913eba496b4cfcc039cd056ffaR1

I'm a bit less familiar with mini test, but this should fixup:

6f281a4

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Perhaps node is missing in CI...?

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Seeing these errors now about missing rollup and esbuild:

/bin/sh: 1: esbuild: not found
error Command failed with exit code 127.
Writing lockfile to /tmp/d20240112-3275-e4xytm/app_aa0b15b827a37c077b0c1857a5812110307500bd/Gemfile.lock

Error:
error https://registry.yarnpkg.com/@rollup/rollup-linux-x64-gnu/-/rollup-linux-x64-gnu-4.9.5.tgz: Extracting tar content of undefined failed, the file appears to be corrupt: "EEXIST: file already exists, mkdir '/home/runner/.cache/yarn/v6/npm-@rollup-rollup-linux-x64-gnu-4.9.5-85946ee4d068bd12197aeeec2c6f679c94978a49-integrity/node_modules/@rollup/rollup-linux-x64-gnu'"
error https://registry.yarnpkg.com/@rollup/rollup-win32-x64-msvc/-/rollup-win32-x64-msvc-4.9.5.tgz: Extracting tar content of undefined failed, the file appears to be corrupt: "ENOENT: no such file or directory, chmod '/home/runner/.cache/yarn/v6/npm-@rollup-rollup-win32-x64-msvc-4.9.5-10491ccf4f63c814d4149e0316541476ea603602-integrity/node_modules/@rollup/rollup-win32-x64-msvc/README.md'"
error Error: ENOENT: no such file or directory, open '/home/runner/.cache/yarn/v6/npm-@rollup-rollup-linux-x64-gnu-4.9.5-85946ee4d068bd12197aeeec2c6f679c94978a49-integrity/node_modules/@rollup/rollup-linux-x64-gnu/.yarn-metadata.json'
/bin/sh: 1: rollup: not found
error Command failed with exit code 127.

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Here is the full error for the Ruby 3.3 rails_7_1_sprockets webpack failure:

Bundle complete! 14 Gemfile dependencies, 81 gems now installed.
Use `bundle info [gemname]` to see where a bundled gem is installed.
         run  bundle lock --add-platform=x86_64-linux
Writing lockfile to /tmp/d20240112-2034-2kkzsa/app_887ca0e996dc7eb969930641cc39b00857a279b8/Gemfile.lock

Error:
error Error: ENOENT: no such file or directory, lstat '/home/runner/.cache/yarn/v6/npm-webpack-5.89.0-56b8bf9a34356e93a6625770006490bf3a7f32dc-integrity/node_modules/webpack/lib'
[webpack-cli] Failed to load '/tmp/d20240112-2034-2kkzsa/app_887ca0e996dc7eb969930641cc39b00857a279b8/webpack.config.js' config
[webpack-cli] Error: Cannot find module 'webpack'
Require stack:
- /tmp/d20240112-2034-2kkzsa/app_887ca0e996dc7eb969930641cc39b00857a279b8/webpack.config.js
- /usr/local/lib/node_modules/webpack-cli/lib/webpack-cli.js
- /usr/local/lib/node_modules/webpack-cli/lib/bootstrap.js
- /usr/local/lib/node_modules/webpack-cli/bin/cli.js
- /usr/local/lib/node_modules/webpack/bin/webpack.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1134:15)
    at Module._load (node:internal/modules/cjs/loader:975:27)
    at Module.require (node:internal/modules/cjs/loader:1225:19)
    at require (node:internal/modules/helpers:177:18)
    at Object.<anonymous> (/tmp/d20240112-2034-2kkzsa/app_887ca0e996dc7eb969930641cc39b00857a279b8/webpack.config.js:2:17)
    at Module._compile (node:internal/modules/cjs/loader:1356:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1414:10)
    at Module.load (node:internal/modules/cjs/loader:1197:32)
    at Module._load (node:internal/modules/cjs/loader:1013:12)
    at Module.require (node:internal/modules/cjs/loader:1225:19) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/tmp/d20240112-2034-2kkzsa/app_887ca0e996dc7eb969930641cc39b00857a279b8/webpack.config.js',
    '/usr/local/lib/node_modules/webpack-cli/lib/webpack-cli.js',
    '/usr/local/lib/node_modules/webpack-cli/lib/bootstrap.js',
    '/usr/local/lib/node_modules/webpack-cli/bin/cli.js',
    '/usr/local/lib/node_modules/webpack/bin/webpack.js'
  ]
}
error Command failed with exit code 2.
E

Error:
WebpackInstallerTest#test_installer_for_webpack:
NoMethodError: undefined method `[]' for nil
    test/installer/webpack_test.rb:15:in `block (3 levels) in <class:WebpackInstallerTest>'
    <internal:kernel>:90:in `tap'
    test/installer/webpack_test.rb:14:in `block (2 levels) in <class:WebpackInstallerTest>'
    test/test_helper.rb:60:in `chdir'
    test/test_helper.rb:60:in `block in with_new_rails_app'
    /opt/hostedtoolcache/Ruby/3.3.0/x64/lib/ruby/3.3.0/tmpdir.rb:99:in `mktmpdir'
    test/test_helper.rb:58:in `with_new_rails_app'
    test/installer/webpack_test.rb:8:in `block in <class:WebpackInstallerTest>'

bin/rails test /home/runner/work/jsbundling-rails/jsbundling-rails/test/installer/webpack_test.rb:7

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

Thought the cache might have something to do with it, no luck.

Will try again in the morning, if I can't figure it out then I will start a new branch without using Appraisal to see if that helps. If it works I will rebase this branch once everything is 🟢

@rmacklin
Copy link
Contributor

Sorry for the dumb question, but are y'all aware of this PR which was opened a few days ago #182 ?

@zzak
Copy link
Member Author

zzak commented Jan 12, 2024

LOL glad that I decided to sleep on that, and sorry I missed Jonathan's PR. 🙇

@zzak zzak closed this Jan 12, 2024
@zzak zzak deleted the test-installer branch January 12, 2024 23:02
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.

3 participants