Update Spring to 3.1.1 - #51022

apologies, I'm going to attempt to answer my own question, but please feel free to correct me if I am mistaken -- it looks like this means that cache_classes may be required when running with spring 3+: rails/spring#652

this seems fine for the normal developer case, but could possibly be worth keeping in mind if you had to run locally without cache_classes for some reason, in which case you might need to disable spring?

in any case, no concerns with merging.

Oh, good callout, thanks! I had glanced at that change, but only to verify that it specifically wants us to set config.cache_classes = false, which we indeed do in development:

However, I failed to realize that we're also using Spring in the test environment, and there we have the value set to true:

Tests still passed, but their documentation explicitly calls out that it's important this be false in the test environment, so I think I have some more investigation to do.

Okay, after some investigation it turns out that the reason they recommend cache_classes = false in the test environment is because otherwise, Spring won't run in the test environment. So it's possible that by following their recommendation we could decrease the time it takes our tests to execute, but unfortunately doing so also causes some tests to fail.

Either way, I think the question of whether or not we want to enable the class cache in the test environment can be discussed independently of this version bump, so I'm going to set it down for now (at least, until the time comes to update Rails to 7.0).

Probably, yeah 🙃

thanks for digging into this, @Hamms! one more question to check my understanding:

the reason they recommend cache_classes = false in the test environment is because otherwise, Spring won't run in the test environment

Can you help me understand what impact this will have on running dashboard unit tests locally -- since we are still going to have cache_classes = true in test, does this mean that bundle exec spring testunit ../path/to/my/test will stop working for developers?

This PR isn't making any changes to our configuration: cache_classes will remain false in test as it has been, and we will continue to not gain any benefits from Spring in that environment until we do change that.

@Hamms I think we may still have a problem here. would you mind double checking what I am seeing?

1. bundle exec spring testunit no longer working locally after pulling latest staging:

Dave-MBP:~/src/cdo/dashboard (staging)$ bundle exec spring testunit test/models/curriculum_reference_test.rb
/Users/dsb/.rbenv/versions/2.7.5/lib/ruby/gems/2.7.0/gems/spring-3.1.1/lib/spring/application.rb:101:in `block in preload': Spring reloads, and therefore needs the application to have reloading enabled.
Please, set config.cache_classes to false in config/environments/test.rb.

2. cache_classes appears to be true in test:

   code-dot-org/dashboard/config/environments/test.rb

   Line 8 in 36fbcfb

   config.cache_classes = true

   
   

Oh, correct, I had the values backwards in my head when I was writing that comment; I meant cache_classes will remain true in test as it has been. You're right that running local tests individually with sprint testunit is no longer working, though. We could recommend developers run individual tests with ruby -Itest instead, but given that we'll have to update this eventually for Rails 7.0 anyway, perhaps we should just figure out why flipping this value breaks tests and get it working now.