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

Add Spring, spring-watcher-listen, and Listen #824

Merged
merged 3 commits into from
Jan 23, 2022

Conversation

nimmolo
Copy link
Contributor

@nimmolo nimmolo commented Jan 18, 2022

Another development-only tool that i'm finding helpful.

Spring is a Rails-maintained application preloader. "It speeds up development by keeping your application running in the background, so you don't need to boot it every time you run a test, rake task or migration."

Listen allows Spring to work more efficiently by sending it notifications of directory changes. It can also be used independently, but this PR does not implement it outside of Spring.

spring-watcher-listen connects the two.

Versions of all three locked to work with Rails 5.2 and each other.

Spring (v4) is, bundled and active by default in Rails 6 (this PR is for Spring v3, the last version that works with Rails 5). It's back to inactive by default in Rails 7 (because "computers are faster" they say), but it's still bundled with Rails 7 currently.

If anyone objects for any reason, no problem. I'm open to using it local-only.

Spring is a Rails-maintained gem for quicker development
Listen allows it to work more efficiently

Versions locked to work with Rails 5.2
@coveralls
Copy link
Collaborator

coveralls commented Jan 18, 2022

Coverage Status

Coverage remained the same at 92.893% when pulling e4cd6f7 on nimmo-add-spring-listen into 9fc48e7 on master.

@JoeCohen
Copy link
Member

I'd love it to have some time shaved off test runs. But I'm seeing some issues:

  1. Will this configuration run on the M1? (Nathan has an M1.)
    See Spring taking minutes to run on MacOS with M1 Chip rails/spring#635 (suggesting upgrading to listen >= 3.3.0; but our Gemfile says gem "listen", ">= 3.0.5", "< 3.2)
  2. Should the following change be made in config/environments/test.rb:
    config.cache_classes = true false
    See https://github.com/rails/spring#enable-reloading
  3. I think it's not working for me. In particular: after a test run spring is not running:
vagrant@ubuntu-focal /vagrant/mushroom-observer (nimmo-add-spring-listen)
$ grep "config.cache_classes" config/environments/test.rb
  # config.cache_classes = true
  config.cache_classes = false
vagrant@ubuntu-focal /vagrant/mushroom-observer (nimmo-add-spring-listen)
vagrant@ubuntu-focal /vagrant/mushroom-observer (nimmo-add-spring-listen)
$ rails t
...
vagrant@ubuntu-focal /vagrant/mushroom-observer (nimmo-add-spring-listen)
$ spring status
Spring is not running.

Compare to https://github.com/rails/spring#usage. (I'll try to see if I'm doing something wrong.)

  1. When I start a test run, I get the following warnings. I don't understand their significance. (And maybe they're related to the fact that Spring isn't running after the tests.):
vagrant@ubuntu-focal /vagrant/mushroom-observer (nimmo-add-spring-listen)
$ rails t
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:49: warning: already initialized constant CollapsibleCollectionOfMappableObjects::PRECISION
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:49: warning: previous definition of PRECISION was here
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:70: warning: already initialized constant CollapsibleCollectionOfMappableObjects::MAX_PRECISION
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:70: warning: previous definition of MAX_PRECISION was here
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:71: warning: already initialized constant CollapsibleCollectionOfMappableObjects::MIN_PRECISION
/vagrant/mushroom-observer/app/classes/map_collapsible.rb:71: warning: previous definition of MIN_PRECISION was here
Started with run options --seed 40014

I don't know why it was locked at 3.1.5, the version of spring-watcher-listen supports < Listen 4.0
@codeclimate
Copy link
Contributor

codeclimate bot commented Jan 18, 2022

Code Climate has analyzed commit e4cd6f7 and detected 0 issues on this pull request.

View more on Code Climate.

@pellaea
Copy link
Member

pellaea commented Jan 18, 2022 via email

@mo-nathan
Copy link
Member

I've merged this into the mo-docker branch and everything seems to be working. Like Joe, I see not speed improvements running the tests, but I also see no downside. I'm fine merging it into master.

@JoeCohen
Copy link
Member

@mo-nathan: Can you run tests in the mo-docker branch?

@JoeCohen
Copy link
Member

That was a dumb question. You said you were running them.

@JoeCohen
Copy link
Member

More important question:
When you run the tests, are you getting the "already initialized constant" warnings? See #824 (comment)
If so, what if anything, should we do about that before merging?
I just switched to master, and am getting the same warnings there.

@JoeCohen
Copy link
Member

It's just me; the "already initialized constant" warnings are just on my desktop, not on my laptop.

@JoeCohen
Copy link
Member

In order to fix this, I had to rebuild my vm, starting from cloning the repo.
Perhaps my problem was due to my following the directions to "springify executables". https://github.com/rails/spring#setup
I'll double-check that tomorrow if I have time.
But right now I'll merge this.

@JoeCohen JoeCohen merged commit 3fd6265 into master Jan 23, 2022
@JoeCohen
Copy link
Member

Using Spring in test environment

  1. The "already initialized constant" warnings were due to my changing config.cache_classes to false in config/environments/test.rb. You have to do that in order to run Spring in that environment. https://github.com/rails/spring#enable-reloading
  2. I won't be using Spring for the tests. Although Spring did improve "user" and "sys" times, it didn't change "real" time.

Without Spring:

$ time rails t
...
Finished in 198.49247s
...
real	3m24.552s
user	2m9.653s
sys	0m42.491s

With Spring:


$ time spring rails t
Running via Spring preloader in process 15117
...
Finished in 195.71937s
real	3m19.578s
user	0m0.251s
sys	0m0.028s

$ time spring rails t
Running via Spring preloader in process 20955
...
Finished in 212.87733s
...
real	3m36.885s
user	0m0.245s
sys	0m0.037s

@nimmolo
Copy link
Contributor Author

nimmolo commented Jan 23, 2022

Hi Joe - sorry to take awhile to get back to these questions, I couldn't remember how i had it set up. I realize it's merged...

  • I'm realizing i didn't run the bundle exec spring binstub --all setup on this branch, so it's unlikely that Spring is active by default on master now
  • When you tested: Did you set it up to use the bin/ prefix before the rails or rake command, or did you set up an .envrc file?

@JoeCohen
Copy link
Member

Neither.
I manually modified config/environments/test.rb.
Then ran time spring rails t.

@nimmolo nimmolo deleted the nimmo-add-spring-listen branch January 29, 2022 03:07
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