search

RefugeRestrooms / refugerestrooms

REFUGE restrooms indexes and maps safe restroom locations for trans, intersex, and gender nonconforming individuals.
http://www.refugerestrooms.org
GNU Affero General Public License v3.0
893 stars 261 forks source link

Upgrade Webpack/Webpacker from 3.x to 4.x #604

Closed DeeDeeG closed 4 years ago

DeeDeeG commented 4 years ago

Webpack/Webpacker 3.x will probably not be supported anymore at some point, and 4.x is included by default with Rails 6.x

Switching helps us stay on the latest dependencies, and stay current with the direction that Rails is going overall. Staying closer to the direction Rails is going should make it easier to stay on actively supported tech/dependencies.

btyy77c commented 4 years ago

I can work on this. Do you also want to upgrade to Rails 6?

DeeDeeG commented 4 years ago

As the most-active maintainer as of late, I'd say that would be awesome. I'm checking in with longer-standing devs/maintainers on Slack, but I personally don't see why not.

Thank you for the offer! (Will keep you posted if I have any updates to give.)

DeeDeeG commented 4 years ago

I am able to review this type of PR and land it, so I'm going to say go for it. Thanks again.

DeeDeeG commented 4 years ago

Do you also want to upgrade to Rails 6?

Yes, I think that would be good, if it proves to not be too much trouble. Only reason why not to (from my perspective) would be performance regressions or bugs/instability. Should be able to catch either during review.

btyy77c commented 4 years ago

Ok. I'll upgrade webpack first and have a different branch for Rails 6.

I'm having a little trouble testing it. All of my branches are failing (including my develop branch: https://travis-ci.org/btyy77c/refugerestrooms/builds/625295028). I think I may be missing an api key to fully test it.

DeeDeeG commented 4 years ago

I'll be able to take a closer look at this tomorrow. Thank you!

DeeDeeG commented 4 years ago

All of my branches are failing (including my develop branch

I'm seeing this too. Sometimes Travis CI gets a reproducible failure on our project that doesn't have anything to do with code changes we've made. These failures that come for no traditional "fault" of our own seem to go away on their own, too.

Speculating: If I had to hazard a guess, I'd say it has to do with Puma running with a maximum 4 threads allowed during testing; In production, if I recall correctly, we are running Puma single-threaded (max 1 thread). I'm not personally familiar with how to change this during testing... And it might not be a correct guess. But that's my guess until that gets ruled out....

Edit: Also, there are some dependencies we don't pin, such as the exact yarn, nodejs or Apt package versions we install during the build. These implicitly get the "latest" versions, some pinned to "within a given major version" (like NodeJS). Theoretically the Travis environment might also update, but usually they leave their base image alone for long stretches of time.

DeeDeeG commented 4 years ago

I tracked down the Travis CI failure by recreating the test conditions manually in Docker.

In summary, the top result for "Mission Creek Cafe" in Google Maps has recently changed, thus breaking our test.

Fuller explanation below, for anyone curious:

(Background information: The Mission Creek Cafe in San Francisco has apparently been closed for some time now. A place called Mission Creek Coffee exists in Washington State. The coffee shop in Washington is now the top result in Google Maps for a generic search for "Mission Creek Cafe." This top result must have changed as of very recently, no earlier than November 1st, when our tests were still passing.)

This test is trying to search for "Mission Creek Cafe" in the search bar, which calls up the Google Maps API's best guess to what location we mean with the search term "Mission Creek Cafe." This now returns a lat/long of the coffee shop in Washington State. (Whereas it used to return the location of the closed coffee shop in San Francisco.)

Google Maps returns a lat/long in Washington, but none of our restrooms in the test db are anywhere near Washington, so the restroom results page is just an empty page. (Our "results view" page simply returns all results within a specific radius of the lat/long passed to it.)

The test expects our testing environment (the CI version of the webapp) to have a result matching our query (effectively a search for restrooms in Washington), but all the restrooms in the testing database are in San Francisco, not Washington.

Thus, the text-match search for the words "Mission Creek Cafe" on the page fails. The test would pass if the "Mission Creek Cafe" restroom entry were visible, but no results are visible at all.

(Trivia note: This sort of thing was briefly looked into in #303 and #311.)