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

possible to set variable not defined explicitly with var? #35

Closed
phishy opened this issue Dec 12, 2014 · 23 comments
Closed

possible to set variable not defined explicitly with var? #35

phishy opened this issue Dec 12, 2014 · 23 comments

Comments

@phishy
Copy link

phishy commented Dec 12, 2014

In some js frameworks, variables are injected into a file that appear to be "global" and have no var definition. I would love to be able to inject variables into a file that are not explicitly defined with var. I can surely look into adding this, was just curious if there is a philosophy difference here.

Example: https://gist.github.com/phishy/c9ac6b2aad7d2f87a5de

@phishy
Copy link
Author

phishy commented Dec 12, 2014

I made a PR here, but weirdly, the try/catch block doesn't work in mocha so the test doesn't pass, but works find in my code. I don't get it.

https://github.com/phishy/rewire/commit/9a459d1e0242cf40368f0b9c51176b58c63658bb

@jhnns
Copy link
Owner

jhnns commented Dec 13, 2014

Sorry, can you give me an example? I don't understand what the problem is 😃

@phishy
Copy link
Author

phishy commented Dec 13, 2014

In MVC frameworks like Sails.js, global variables exist inside modules that are not defined in that module. I would like to be able to rewire those, however, rewire didn't allow injecting variables that weren't defined with var. This PR allows you to introduce new variables into a module that weren't defined with var in that module. Something like this: https://gist.github.com/phishy/e135bdddeaf176bd5a71

jhnns added a commit that referenced this issue Dec 14, 2014
@jhnns
Copy link
Owner

jhnns commented Dec 14, 2014

rewire didn't allow injecting variables that weren't defined with var.

That works, I've added a test-case for it.

The problem with implicit global vars is though, that they will be changed globally by rewire. Thus writing

Product.__set__({ Vendor: vendorStub });

is effectively the same as

Vendor = vendorStub;

That's because rewire prepends a list of var statements that import global vars to the local scope. To create this we need to know all globals before the module is executed, but implicit global vars are created as the module is executed.

However, I don't think that is a great problem. When the module to test uses implicit global vars you don't need rewire to change them. rewire has been designed to be used with a module system.

@phishy
Copy link
Author

phishy commented Jan 2, 2015

My change is still working locally, however, I believe Johannes fixed it
with a commit as well. Make sure you're on the right branch and you should
be good to go.

On Fri, Jan 2, 2015 at 2:10 AM, cdnadmin notifications@github.com wrote:

@phishy https://github.com/phishy did this ever work for you? I'm
having a similar issue with an AWS Lambda function. "context" is defined in
the aws env, so, my rewired tests are not working due to "context is not
defined" error.


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns jhnns closed this as completed Jan 3, 2015
@phishy
Copy link
Author

phishy commented Feb 9, 2015

@jhnns I'm sorry. I'm just now realizing that you added a test case but never added a fix like the one I had on my pull request. I'm just now realizing as I recloned my project and lost my changes to rewire.

Let me try to explain further. Using the Sails.js framework, all Models and Services are automatically added to global scope. Therefore when I require a file in order to test it, there are variables being used in that file that were never defined within it (with or without var). This is why I created the pull request that allows overwriting variables that are attached to global.

I understand that I'm requesting a feature in rewrite to overcome deficiencies in poorly designed frameworks, however, I don't know enough about rewire to understand if adding this feature would have side effects. Otherwise, it would allow mocking and testing object in Sails.js and frameworks like it that hang some objects onto global.

https://github.com/phishy/rewire/commit/9a459d1e0242cf40368f0b9c51176b58c63658bb

@jhnns
Copy link
Owner

jhnns commented Feb 9, 2015

Sorry, but wrapping eval() with try/catch is not a valid option. The method call should fail if the variable is not defined.

If the variable has been added by Sails.js to the global scope before you call __set__, everything should work fine. When there is no global variable yet, rewire will throw a ReferenceError which is the desired behavior.

Could you describe your actual problem? Is there an error? Does mocking not work?

@phishy
Copy link
Author

phishy commented Feb 10, 2015

When I am mocking the model, none of the variables that would be set by
Sails are there, hence undefined. Rewire fails to set them as well even if
I call set.
On Feb 9, 2015 6:18 PM, "Johannes Ewald" notifications@github.com wrote:

Sorry, but wrapping eval() with try/catch is not a valid option. The
method call should fail if the variable is not defined.

If the variable has been added by Sails.js to the global scope before
you call set, everything should work fine. When there is no global
variable yet, rewire will throw a ReferenceError which is the desired
behavior.

Could you describe your actual problem? Is there an error? Does mocking
not work?


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns
Copy link
Owner

jhnns commented Feb 10, 2015

Could you set up a tiny example project where the problem exists? It's hard to tell the issue when I don't know enough about Sails.

@phishy
Copy link
Author

phishy commented Feb 10, 2015

No problem. It's really just about testing files and mocking any globals
that would be injected by requiring another file, or being required by
another file.

Here is a demo.
https://ide.c9.io/phishy/sails

When running the Mocha test, you can see that it fails to overwrite the
Story variable, because it is not available outside of context when not
loading up the framework. In this case I am only trying to mock out all of
the external dependencies.

On Tue, Feb 10, 2015 at 9:24 AM, Johannes Ewald notifications@github.com
wrote:

Could you set up a tiny example project where the problem exists? It's
hard to tell the issue when I don't know enough about Sails.


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns
Copy link
Owner

jhnns commented Feb 11, 2015

You need to initialize Sails at least to test your modules. Rewire can't mock anything when it is not defined.

@phishy
Copy link
Author

phishy commented Feb 11, 2015

I'm already successfully mocking everything and its working great. I guess
I'll just use my fork. Thanks for your help. It seems within reason to me,
that rewire should be able to inject undefined variables.
On Feb 10, 2015 7:45 PM, "Johannes Ewald" notifications@github.com wrote:

You need to initialize Sails at least to test your modules. Rewire can't
mock anything when it is not defined.


Reply to this email directly or view it on GitHub
#35 (comment).

@phishy
Copy link
Author

phishy commented Feb 11, 2015

Its incredibly expensive to load Sails, hence the mocking, plus it's
cleaner, IMO.
On Feb 10, 2015 7:45 PM, "Johannes Ewald" notifications@github.com wrote:

You need to initialize Sails at least to test your modules. Rewire can't
mock anything when it is not defined.


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns
Copy link
Owner

jhnns commented Feb 11, 2015

How does your fork solve your problem? You're just wrapping the eval() with try/catch...

You just need to define the variables to mock on the global object and everything works fine. From the rewire point of view it doesn't make sense to mock global variables with rewire.

@phishy
Copy link
Author

phishy commented Feb 11, 2015

It solves my problem by allowing me to inject global variables into the
script. I know, it's a stupid argument, but some of the major frameworks
operate this way, and there's no reason to load the framework, and test it
when I"m writing userland code.

It solves the problem, because the patch actually allows writing a global
variable to succeed instead of throwing an exception.

On Wed, Feb 11, 2015 at 8:47 AM, Johannes Ewald notifications@github.com
wrote:

How does your fork solve your problem? You're just wrapping the eval()
with try/catch...

You just need to define the variables to mock on the global object and
everything works fine. From the rewire point of view it doesn't make sense
to mock global variables with rewire.


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns
Copy link
Owner

jhnns commented Feb 11, 2015

Sorry, that is just not true. The eval() throws an exception and the error is ignored, no variable is set on the global scope. Your provided test-case is checking for the wrong value, moduleFake.getValue() returns myValue, not undefinedValue.

@phishy
Copy link
Author

phishy commented Feb 11, 2015

Let me correct myself. It doesn't allow me to add variables onto global.*,
it allows me to set any variable within the head of the script (ones not
explicitly defined with var). I will try to boil up another test case that
makes more sense, as it seems it is hard to explain how this change is
currently working for me.

On Wed, Feb 11, 2015 at 10:51 AM, Johannes Ewald notifications@github.com
wrote:

Sorry, that is just not true. The eval() throws an exception and the
error is ignored, no variable is set on the global scope. Your provided
test-case is checking for the wrong value, moduleFake.getValue() returns
myValue, not undefinedValue.


Reply to this email directly or view it on GitHub
#35 (comment).

@jhnns
Copy link
Owner

jhnns commented Feb 12, 2015

I will try to boil up another test case that makes more sense

That would be great 😀

@phishy
Copy link
Author

phishy commented Feb 16, 2015

Here is a test case for wanting to be able to set undefined variables. https://gist.github.com/phishy/cb06c276a332b01ec149

@jhnns
Copy link
Owner

jhnns commented Feb 17, 2015

I'm so sorry. I just didn't get what you were trying to say 😁

Should be solved with 2.3.0

@phishy
Copy link
Author

phishy commented Feb 17, 2015

Thanks.

On Mon, Feb 16, 2015 at 7:16 PM, Johannes Ewald notifications@github.com
wrote:

I'm so sorry. I just didn't get what you were trying to say [image:
😁]

Should be solved with 2.3.0


Reply to this email directly or view it on GitHub
#35 (comment).

@mathieumg
Copy link

I'm pretty sure I have a use case for this too, thanks!

@arsal123
Copy link

arsal123 commented Jan 6, 2020

Same here. I m using rewire though not sure if there is any other latest plugin that is better for mocking and checking internal variables during unit tests

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

No branches or pull requests

4 participants