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

Test Helpers in Factory break development build #247

Closed
patocallaghan opened this issue Sep 9, 2016 · 16 comments
Closed

Test Helpers in Factory break development build #247

patocallaghan opened this issue Sep 9, 2016 · 16 comments

Comments

@patocallaghan
Copy link
Collaborator

patocallaghan commented Sep 9, 2016

Since the new development mode has shipped I notice that factories are being included in the development build in the app's JS by default and even when I set ENV.factoryGuy = false, i.e. I don't want to use Factory Guy in development mode.

In one of my factories I import a test helper like so:

import { getAdminFixture } from 'embercom/tests/helpers/data';

but because the factory is being included in dev build and the test helpers aren't, it breaks my app in development mode with the following error: Could not find module embercom/tests/helpers/data imported from embercom/tests/factories/engage/message-variation

Should we not be excluding the factories from development builds if you don't want to use the new development mode?

@danielspaniel
Copy link
Collaborator

Oooo .. you are very right .. those factories should not be in dev if you not using them.
Great bug by the way. You always find some really interesting ones.
I will get my elves to work on that tonight, so you can be back in business tomorrow AM.

@danielspaniel
Copy link
Collaborator

This was good fix. Should have thought of this before.

Fixed in https://github.com/danielspaniel/ember-data-factory-guy/releases/tag/v2.7.8

@GavinJoyce
Copy link

GavinJoyce commented Nov 30, 2016

@danielspaniel this commit seems to have reintroduced this issue

@danielspaniel
Copy link
Collaborator

I think I was wrong @GavinJoyce to say that the factories should not be included in dev build because they still need to be for the ember test runner which runs when you go to browser and use the /tests route.
But maybe I am wrong about that because why would ember test loader run tests and not include all the test files automatically?
Can you confirm that I am not loosing my noodle, and that when you run things in /tests url that this is development environment and that all test files/helpers are available.
When are you finding this issue? Is it when you run ember t ( test environment ) ... that would make no sense since all test files are also included .. so I am a bit puzzled.

@GavinJoyce
Copy link

Thanks @danielspaniel. I'm not fully sure of the original context to this issue, just that it's reoccurring in our app since updating ember-data-factory-guy. @patocallaghan has more context, he's going to look into the details and post more info when he has it

@patocallaghan
Copy link
Collaborator Author

@danielspaniel we are seeing this issue when we load our app normally in development. We don't want to use FactoryGuy in our local apps.

You are right that FactoryGuy should load if environment == 'test' or if hitting tests/index.html && environment == 'dev'. But by default FactoryGuy shouldn't be included in dev. According to that change above it is always opting the dev environment into including FactoryGuy. It's bypassing the factoryGuyEnabled config option.

@danielspaniel
Copy link
Collaborator

Ok @patocallaghan.
So I think what is causing problem is I am putting the factory guy files into the application tree.
For people that want to use factory guy in dev that is fine, but for people that don't I have to put those factory guy files in the test tree ( tests.js )
I recall ( sort of ) that there was a problem with that before but I will try again and see if that works.

@danielspaniel
Copy link
Collaborator

@patocallaghan try https://github.com/danielspaniel/ember-data-factory-guy/releases/tag/v2.9.7
I tested it quite a bit, but I am still nervous. This is squirrelly issue until I can think of how to write some tests to make sure it's spot on.

@GavinJoyce
Copy link

@danielspaniel thanks, that works in our app

@danielspaniel
Copy link
Collaborator

Great, glad that worked @GavinJoyce.

@patocallaghan
Copy link
Collaborator Author

Thanks @danielspaniel 🙇

@danielspaniel
Copy link
Collaborator

your welcome @patocallaghan .. glad it's working out ( nice emoji .. me like )

@danielspaniel
Copy link
Collaborator

danielspaniel commented Jan 18, 2017

hey @patocallaghan .. this issue is returning to haunt me.

Someone setup a new project with latest factory guy and it was total disaster ( since acceptance tests kept barfing ) .. and I finally figured out it was this issue again.

Question for you:

Because you have references to other test files in factories

is there a way that you could put all your "helper" files that you import into factories
into one directory like tests/utils or tests/fg-helpers
anything you like really ..

because I am needing to go back to this:

  includeFactoryGuyFiles: function() {
    var includeFiles = false;
  
    // changed to /test/ to not include in development, but that not working so well 
    if (this.app.env.match(/test|development/)) {
      includeFiles = true;
    } else {
      includeFiles = this.factoryGuyEnabled;
    }

    return includeFiles;
  },

since if I don't include the files in development running an acceptance tests with localhost:port/tests will not load the factories ( since the includeFactoryGuyFiles function is returning false in development mode. )

Not sure how this ever worked for anyone frankly. But the change was in 2.9.7 so maybe not too many people upgraded, or they were using factoryGuy: true in environment.js ( development section )

But need to come up with a solution asap.

  treeForApp: function(appTree) {
    var trees = [appTree];

    if (this.includeFactoryGuyFiles()) {
      try {
        if (fs.statSync('tests/factories').isDirectory()) {
          var factoriesTree = new Funnel('tests/factories', {
            destDir: 'tests/factories'
          });
          var utilsTree = new Funnel('tests/place-where-my-helper-functions-are', { 
            destDir: 'tests/place-where-my-helper-functions-are'   //  
          });
          trees.push(factoriesTree);
          trees.push(utilsTree);
        }
      } catch (err) {
        // do nothing;
      }
    }

    return mergeTrees(trees);
  },

So, basically I need a way to include factory guy still in development, so it works in the /tests mode
and then also import any files that factories might use. If you can put them in one directory ( suggest me a name ) that would solve the problem.

I can think of no other alternative. Can you? I am open to any and all ideas.

@patocallaghan
Copy link
Collaborator Author

@danielspaniel just to confirm that 2.11.0 works for us when we set:

ENV.factoryGuy = { enabled: false, useScenarios: false };

Factory Guy doesn't appear in our dev environment. Thanks for making those changes 👍

@danielspaniel
Copy link
Collaborator

Thanks for confirming .. @patocallaghan 😃

@rreckonerr
Copy link

ENV.factoryGuy = { enabled: false, useScenarios: false };

For v2.13.27 dev build works well, but if you navigate to /tests most likely you'll find your test suit broken.
Upgraded to v3.9.7 (latest ATM) and it works good for /tests on a dev build route too.

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