Add configurability for external Cypress project through additional environment variable #159
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…SS_DIR inspect to config class
dmanliclic715
commented
Feb 8, 2024
lib/cypress-rails/finds_bin.rb
Outdated
| @@ -4,8 +4,8 @@ module CypressRails | |||
| class FindsBin | |||
| LOCAL_PATH = "node_modules/.bin/cypress" | |||
|
|
|||
| def call(dir = Dir.pwd) | |||
| local_path = Pathname.new(dir).join(LOCAL_PATH) | |||
| def call(cy_dir = Dir.pwd) | |||
There was a problem hiding this comment.
Not entirely sure these variable name changes are necessary, but I just wanted it to be more clear that we're looking for a bin path within a directory with Cypress.
searls
requested changes
Feb 8, 2024
lib/cypress-rails/config.rb
Outdated
| @@ -2,17 +2,19 @@ | |||
|
|
|||
| module CypressRails | |||
| class Config | |||
| attr_accessor :dir, :host, :port, :base_path, :transactional_server, :cypress_cli_opts | |||
| attr_accessor :dir, :cy_dir, :host, :port, :base_path, :transactional_server, :cypress_cli_opts | |||
There was a problem hiding this comment.
I think I'd rather spell these out as :rails_dir and :cypress_dir for clarity
There was a problem hiding this comment.
Makes sense! Will add!
README.md
Outdated
| @@ -156,6 +156,7 @@ preferred environment variables project-wide using a tool like | |||
|
|
|||
|
|
|||
| * **CYPRESS_RAILS_DIR** (default: `Dir.pwd`) the directory of your project | |||
There was a problem hiding this comment.
Suggested change
| * **CYPRESS_RAILS_DIR** (default: `Dir.pwd`) the directory of your project | |
| * **CYPRESS_RAILS_DIR** (default: `Dir.pwd`) the directory of your Rails project |
dmanliclic715
force-pushed
the
add-cypress-dir-environment-variable
branch
from
aa22a9b to
3a4a99d
Compare
dmanliclic715
commented
Feb 8, 2024
| @@ -19,8 +19,8 @@ class Init | |||
| }) | |||
| JS | |||
|
|
|||
| def call(dir = Dir.pwd) | |||
| config_path = File.join(dir, "cypress.config.js") | |||
| def call(cypress_dir = Config.new.cypress_dir) | |||
There was a problem hiding this comment.
@searls
Needed to also include this here, to make sure we're looking to write the "cypress.config.js" to the cypress dir. Let me know if that checks out!
|
Is this ready for merge or is there more to do? |
|
@searls All good on my end! Thanks again for taking a look! |
|
Ok. Two things:
|
…S_RAILS_CYPRESS_DIR to be same value as rails_dir attribute
dmanliclic715
force-pushed
the
add-cypress-dir-environment-variable
branch
from
66bb871 to
ae2a607
Compare
Thank you! |
|
Thanks @dmanliclic715 -- released as 0.7.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi!
Big
cypress-railsfan 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-railscommands. With how the mainCYPRESS_RAILS_DIRenv 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
LaunchesCypressservice. Let me know if this makes sense or if there's anything else I can change!