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 regression test for escaping feature names #378

Merged
merged 1 commit into from
Oct 5, 2018

Conversation

AlexWheeler
Copy link
Collaborator

@AlexWheeler AlexWheeler commented Oct 2, 2018

@jnunemaker great idea about adding a regression test lemme know what ya think

  • 7151715 was merged which escapes
    feature_names like we were doing before the change to support slashes
    and adding the FeatureNameFromRoute module for pulling the feature name
    out of the request. This adds a regression spec to make sure this
    continues to work in the future as changes are made.

  • I decided to test this in a separate describe block since the current action_subclass is super simple and really easy to understand what its testing. Since the FeatureNameFromRoute returns a request body with values and includes a module, etc I think its nice to separate the two. Since feature_name is a private method I didn't really want to test it using send and instead decided to run the fake request via the public run method like the other specs have. If you like this approach I'll add the same for Api::Action just lemme know. Totally open to other suggestions to better test this too

  • I decided to stub path_info instead of add the header PATH_INFO to env because that feels like messing with internals of the path_info method and we just care about the public method and not how its implemented under the hood

@AlexWheeler AlexWheeler force-pushed the regression-test-for-escaping-feature-names branch 5 times, most recently from 369a8c8 to b5ce3b3 Compare October 2, 2018 03:23
* 7151715 was merged which escapes
feature_names like we were doing before the change to support slashes
and adding the FeatureNameFromRoute module for pulling the feature name
out of the request.  This adds a regression spec to make sure this
continues to work in the future as changes are made.
@jnunemaker jnunemaker merged commit 117d0e9 into master Oct 5, 2018
@jnunemaker jnunemaker deleted the regression-test-for-escaping-feature-names branch October 5, 2018 00: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.

2 participants