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

Environment is "development" instead of "test" during tests #32

Open
viniciussbs opened this issue Mar 23, 2019 · 14 comments
Open

Environment is "development" instead of "test" during tests #32

viniciussbs opened this issue Mar 23, 2019 · 14 comments

Comments

@viniciussbs
Copy link

Hi! I don't know if this is a bug here or maybe on FastBoot - or maybe I am missing something. But this code is evaluating "development" instead of "test":

let config = getOwner(this).resolveRegistration('config:environment');
config.environment // => "development"

This happens only in FastBoot tests. In acceptance tests it evaluates "test".

@ryanto ryanto added the bug label Apr 2, 2019
@ryanto
Copy link
Member

ryanto commented Apr 18, 2019

Hey @viniciussbs - Quick question: How are you running the tests for you application?

@viniciussbs
Copy link
Author

Hi! ember s and then https://localhost:4200/tests?hidepassed, because I was debugging the tests. I was following your tip of running kill -USR1 <pid> to put node into debug mode.

@ryanto
Copy link
Member

ryanto commented Apr 19, 2019

Ok great. In trying to track this one down I noticed that if you run ember test or ember test --server you'll get the right environment.

I'm waiting to hear back on if we can have the /tests URL use a FastBoot app with a test env.

@ryanto
Copy link
Member

ryanto commented Apr 23, 2019

Ok quick follow up!

Because you're running ember s and visiting /tests URL you're getting the FastBoot output of a development app, so it has its env set to development. The best way to ensure you have a test env is to use ember test --server.

In order to get this fixed I believe it will require a change to FastBoot. We need a way to tell FastBoot to run with a different environment configuration from what it was built with. My guess is this will require a change to FastBoot itself.

I'm going to close this discussion because I'm not sure there's anything this library can do right now to move this forward. There's an issue over here in FastBoot to track this: ember-fastboot/fastboot#218

@ryanto ryanto closed this as completed Apr 23, 2019
@viniciussbs
Copy link
Author

Nice! I usually run ember test --server, but at that time I was switching between tests and development, so that's why I was visiting /tests.

Thank you!

@jkeen
Copy link
Collaborator

jkeen commented Jan 17, 2020

Spent some time trying to track this down and crawled through the fastboot source yesterday, and figured out a way to make this work right now in the fastboot-testing.js setupFastboot hook.

//config/fastboot-testing.js
let config = require('./environment');

module.exports = {
  resilient: false,
  sandboxGlobals: {},
  setupFastboot: fastbootInstance => {
    let oldConfig;
    if (process.env.APP_CONFIG) {
      oldConfig = process.env.APP_CONFIG;
    }

    process.env.APP_CONFIG = JSON.stringify(config('test'));
    fastbootInstance.reload()

    if (oldConfig) {
      process.env.APP_CONFIG = oldConfig;
    }
    else {
      delete process.env.APP_CONFIG
    }
  }
};

Basically what's happening is when the ember app gets built all the current environment config gets stuffed into the package.json file under "AppConfig", which is where it gets read out of when the app gets built in Fastboot. Relevant lines:

https://github.com/ember-fastboot/fastboot/blob/1d0e4c86f4f3bdecc9acc95116c720db17c77a46/src/ember-app.js#L45-L51

So all we need to do from this addon is set the APP_CONFIG environment variable to the stringified configuration we want and reload the fastboot instance so it takes. We can do that in the consumer app's setupFastboot hook, but it's less efficient than it could be because before that hook is called we're calling reload() already from this addon:

https://github.com/embermap/ember-cli-fastboot-testing/blob/7cb4b4705498485dc6c977bf33bbb6b6fc25cf38/lib/helpers.js#L66-68

But this should get @viniciussbs going until a real fix can be put in. And good news: probably no need to modify fastboot? What do you think, @ryanto?

@jkeen jkeen reopened this Jan 17, 2020
@ryanto
Copy link
Member

ryanto commented Jan 17, 2020

Oh awesome nice find!

  1. Is process.env.APP_CONFIG public API? Could we box ourselves into a bad situations by overriding that?

  2. The whole setupFastboot hook was added for folks that have really complicated or special FastBoot setups. Does this qualify as a special setup? In other words, how would you feel about fastboot-testing not supporting this out-of-the-box, but instead having instructions in the docs for using /tests?

  3. If we did add this to setupFastboot should we have a way to super it? Or should folks that want to add their own setupFastboot now be responsible for copying over our APP_CONFIG code?

@jkeen
Copy link
Collaborator

jkeen commented Jan 17, 2020

  1. There's a test for it in the fastboot source and it's preserved in the 3.x fastboot work @rwjblue is doing, so I think it's cool to use?

  2. I don't think this fix constitutes a special setup—rather this is fixing a strange quirk. "I want my application tests to run in development mode but only when I visit my app at localhost:4200/tests and not at localhost:7357" feels like the time you'd say "that's a special setup", in my opinion.

With more people using that mockServer to do full blown acceptance testing with network requests, I'd imagine more people would be perplexed by that issue due to the common pattern of setting env vars for network URLs in the application environment config. The way this bug presented itself to me was the fastboot app loading data from development URLs instead of test data.

  1. What kind of special fastboot setup have you seen people do in that setupFastboot hook? (I ran into another issue on this same project about trying to mock dates using timekeeper but it didn't seem like I could do it in the setupFastboot hook. That's a separate topic)

@ryanto
Copy link
Member

ryanto commented Jan 21, 2020

Ok cool! I'm on board unless anyone has any objections.

I guess we could move the process.env.APP_CONFIG code to happen as we are setting up Fastboot, so there's no need to reload it in setupFastboot?

Also, once that app is booted should we reset process.env.APP_CONFIG to its original value? I'd imagine ember-cli-fastboot also relies on this code as well, so keeping it as a test env might cause issues?

@jkeen
Copy link
Collaborator

jkeen commented Jan 21, 2020

Well I spoke too soon and ran into a snag with this hack approach that maybe our fix can overcome?

This setupFastboot hook in fastboot-testing gets run in development mode? When starting up the app in dev mode, the fastboot config has the test envrionment's settings which causes problems. Oof

So I guess we do need to be really careful about that env var in making sure to reset it after it's done what we need it to

@jkeen
Copy link
Collaborator

jkeen commented Jan 21, 2020

Updated my code snippet above (#32 (comment)) to account for the above problem and it's working as it should now. Still not sure why that hook loads in dev mode though

@ryanto
Copy link
Member

ryanto commented Jan 21, 2020

Great!

If we're going to bring this into Fastboot testing does it make sense to move the env logic into this function (https://github.com/embermap/ember-cli-fastboot-testing/blob/master/lib/helpers.js#L73) so that we don't need to call reload?

Any downsides?

@jkeen
Copy link
Collaborator

jkeen commented Jan 22, 2020

That seems good, I think we might need to do it again before the fastboot.reload() in the reloadServer function too, though, won't we? I think fastboot.reload() rebuilds the app doesn't it?

@ryanto
Copy link
Member

ryanto commented Jan 24, 2020

Yup nice catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants