search

testdouble / cypress-rails

Helps you write Cypress tests of your Rails app
Other
321 stars 48 forks source link

Add configurability for external Cypress project through additional environment variable #159

Closed dmanliclic715 closed 9 months ago

dmanliclic715 commented 9 months ago

Hi!

Big cypress-rails fan and first time open source contributor here, so please excuse any open source contribution etiquette I'm missing!

Anyways, my team has a specific use-case that isn't quite covered(I think) by the current functionality of this gem, so I went about attempting to add that in and have found that it's working as expected. I'm not sure if others would benefit from the same functionality, but I wanted to put a PR out there to get some feelers.

Essentially, our Rails application serves as the API for a React/Cypress project that exists outside of our Rails directory. So, I needed a way for our external cypress project to be launched via the cypress-rails commands. With how the main CYPRESS_RAILS_DIR env var is configured, it seems like there's an assumption that the Cypress project also exists within the same directory. So I figured it'd be best to spin up another configurable environment variable that specifies where our Cypress project is located called: CYPRESS_DIR.

We'd then use that specific variable to manage the launching of Cypress in the LaunchesCypress service. Let me know if this makes sense or if there's anything else I can change!

searls commented 9 months ago

Is this ready for merge or is there more to do?

dmanliclic715 commented 9 months ago

@searls All good on my end! Thanks again for taking a look!

searls commented 9 months ago

Ok. Two things:

  1. All other env vars start with CYPRESS_RAILS so the new property should, too
  2. Significantly, this looks like it would be a breaking change to anyone currently setting CYPRESS_RAILS_DIR because they wouldn't be setting CYPRESS_DIR. That means if I currently have both in foo/, after merging this PR it would change to Dir.pwd. The cypress dir should default to whatever rails_dir is, not Dir.pwd, to avoid this
  3. Possible to write a test for this? The fact I caught this visually worries me because I don't use the library
dmanliclic715 commented 9 months ago
  1. Just tacked on the CYPRESS_RAILS prefix to the env var and it now reads CYPRESS_RAILS_CYPRESS_DIR
  2. Made the default value for the Env.fetch("CYPRESS_RAILS_CYPRESS_DIR", default: rails_dir) rails_dir to default to whatever the set value of CYPRESS_RAILS
  3. Added some test to test that CYPRESS_RAILS_CYPRESS_DIR defaults to the same value as CYPRESS_RAILS_DIR.

Thank you!

searls commented 9 months ago

Thanks @dmanliclic715 -- released as 0.7.0