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

disable(), enable(), and destroy() methods for mocks #228

Merged
merged 1 commit into from
Aug 5, 2016

Conversation

ryedeer
Copy link
Contributor

@ryedeer ryedeer commented Aug 5, 2016

These methods can be used to temporarily disable and re-enable the mock or to completely destroy the mockjax handler.

The excerpt from the docs:

  • Use method disable() to temporarily disable the mock. You can re-enable
    the disabled mock using enable().
  • Use method destroy() to completely remove the mockjax handler for the mock.
    The isDestroyed property is set to true when the mock is destroyed.

@danielspaniel
Copy link
Collaborator

danielspaniel commented Aug 5, 2016

What is a good use case for this feature? Me curious. Can't think of one off hand.
Why would you set a mock and then disable it?

@youroff
Copy link
Contributor

youroff commented Aug 5, 2016

For example if you need to mute global mockFindAll temporarily in favor of mockQuery.
It maybe pretty rare case, but we ran into it right away. Also this extension doesn't cost anything, so why not to have it?

Besides, it's natural to expect that if you can create the mock, then you should be able destroy it (or turn off somehow).

@danielspaniel
Copy link
Collaborator

I know I sound like an a-hole .. sorry ..

What I am really trying to say .. is that I suspect you are doing things that are kinda whacky ( no offence ) and then fixing factory guy to cover for that.

I suspect you are setting up tons of mocks up front and then running tests.
I am not 100% sure you doing this.. but I suspect it.
This is ( if you are doing that ) .. not a great practice ( though if you love it, I don't want to stop you from rocking out on fun things ) .. best practice is to have test that does one or two things, mock those and move on.
So really I just trying to help to make stuff better in your tests.
I would be happy to check out your code and tests and help you organize things because indeed things could be slow if you doing that ( mocking tons of things up front ) .. and I believe there is better way.

@ryedeer
Copy link
Contributor Author

ryedeer commented Aug 5, 2016

Suppose we have an application that loads a user index at every launch, so it seems natural to define mockFindAll('user') at the setup of moduleForAcceptance. But suppose there's a certain feature that needs to load the user index with additional parameters, so we may want to define something likemockQuery('user', { type: 'VIP' }) in a particular test. But it won't work because the mockFindAll defined earlier will catch all those requests. Of course, we could move this test in a separate group of tests and define that mockFindAll in all test groups except that one, but I think it would be more convenient to disable it at the start of that special test.

@youroff
Copy link
Contributor

youroff commented Aug 5, 2016

No we're not doing anything like that, of course ;) And I agree that it's possible to live without it, but I'm 100% sure that these features would make this tool better, otherwise I wouldn't propose it as PR.

Here are my thoughts about this:

  1. If you don't need some mocks anymore, removing them from mockjax makes lookup faster.
  2. I used to think that in good API if you can create something, then you should have an ability to destroy it as well.
  3. Enabling/disabling is not obvious advantage, but it can definitely make tests cleaner. I will think about use cases and show them later.

Anyways, you're free to do anything you want with this PR obviously. If we think that it's critical for us, we'll use fork or just monkeypatch it.

@danielspaniel
Copy link
Collaborator

Boy, I would love to see your tests.

And, I don't mind merging this.

I just want to make 100% sure I understand why you need it, so I can help to make sure you using FG properly.

For example, mockTeardown in the after hook will teardown all mocks. Done.

So I just want to see a use case for this .. maybe I learn something new .. and I start using this new feature myself ( happens all the time )

Just give me one use case .. and I merge right away

@danielspaniel
Copy link
Collaborator

danielspaniel commented Aug 5, 2016

Hold on .. I am reading the use case ( did not see before )

Ok .. I got it ..

That is an interesting case. And it is a use case, so I will merge it.

And this is one everyone probably can relate to. You always have to load up the user/users at the start of application.

Putting in moduleForAcceptance makes sort of sense.

But I would not recommend that.

I have been using a concept where I create test scenario files ( that hold the data / mock the data for any given page ) sort of like page object files for each page.

// sample page object  ( ember-cli-page-object ) file => tests/pages/links.js

import PageObject from 'ember-cli-page-object';
import {categories} from '../utilities';

let { is, text, visitable } = PageObject;

export default PageObject.create({
  categories,
  visit: visitable('/account/links'),

  allLinks: {
    scope: '.links-selector',
    headers: text('.header', { multiple: true }),
    allNames: text('.t-category-name, .t-link-name', { multiple: true }),
    salesChecked: is('checked', '.header[data-category=SALES] input'),
    serviceChecked: is('checked', '.header[data-category=SERVICE] input')
  },

  myLinks: {
    scope: '.my-links-list',
    names: text('li p', { multiple: true })
  }
});
// sample scenario file => tests/scenarios/links.js

import {Scenario}  from 'ember-data-factory-guy';
import User from '../user';

export default class extends Scenario {

  run() {
    this.include([User]);
    this.makeData();
    return this;
  }

  makeData() {
    let sales = this.make('category', 'sales');
    let service = this.make('category', 'service');
    this.link1 = this.make('link', { categories: [sales, service] });
    this.link2 = this.make('link', { categories: [service] });
    this.link3 = this.make('link', 'unselected', { categories: [service] });
  }
}

and the acceptance test:

import moduleForAcceptance from '../../helpers/module-for-acceptance';
import page from '../../pages/account/links';
import Scenario from '../../scenarios/account/links';

let scenario;
moduleForAcceptance('Acceptance | Account | Links', {
  beforeEach() {
    scenario = (new Scenario()).run();
  }
});

test('visting for first time shows link categories, and links correctly', function() {
  page.visit();

  andThen(()=> {
    equal(page.categories.isActive, 'All');
    deepEqual(page.categories.list, ['All','SALES','SERVICE']);
    deepEqual(page.allLinks.allNames, ["SALES", "Link-1", "SERVICE", "Link-1", "Link-2", "Link-3"]);
    deepEqual(page.myLinks.names, ["Link-1", "Link-2"]);
  });
});

@danielspaniel danielspaniel merged commit 0643e08 into adopted-ember-addons:master Aug 5, 2016
@youroff
Copy link
Contributor

youroff commented Aug 5, 2016

Thanks. We just started moving our tests to FG, right now they are very basic. But next time we'll try to provide better example with some feature proposals.

@danielspaniel
Copy link
Collaborator

Sure, I'd be happy to help out and give any suggestions I can.
If you can move to this style I just oulined ( above ) . You will be loving life and your tests too. I just started using Scenario class for test scenarios and it is just way too much fun.

Here is another sample that have more mockQuery stuff:

// tests/scenarios/wsg-cases.js
import FactoryGuy, {Scenario}  from 'ember-data-factory-guy';
import VehicleSearch from 'portal-ui/scenarios/service/vehicle-search';
import Wsg from 'portal-ui/scenarios/service/wsg';
import {flatten} from 'portal-ui/utils/array-ext';
import User from '../user';

export default class extends VehicleSearch(Wsg) {

  run() {
    this.include([User]);
    this.makeAgents();
    this.mockCreate('service/wsg-case');
    return this;
  }

  makeAgents() {
    this.agents = this.makeList('service/wsg/agent', 8);
  }

  findCases({num=1, params={}, traits=[]}={}) {
    let args = flatten(['service/wsg-case', num, traits]);
    let cases = this.buildList.apply(FactoryGuy, args);
    return this.mockQuery('service/wsg-case', params).returns({ json: cases });
  }
}
// tests/acceptance/wsg-tests.js

import moduleForAcceptance from '../../helpers/module-for-acceptance';
import page from '../../pages/wsg-cases';
import Scenario from '../../scenarios/wsg-cases';

let scenario;
moduleForAcceptance('Acceptance | Service | Wsg Home Office | Search Cases', {

  beforeEach() {
    scenario = (new Scenario()).run();
    scenario.setHomeOfficeUser();
  }
});

test('Can search with agents and dealer code', async function() {
  scenario.findCases({num: 1});
  await page.visitHO();

  let agent = scenario.agents[0];
  await page.filter.dealerCode.enter('DCODE');
  await page.filter.agents.select(agent.get('name'));

  await page.filter.doSearch();

  ok(page.results.number, 1);
});

@danielspaniel
Copy link
Collaborator

What I trying to show here is that I would not do what you are doing .. I would use scenarios to set up data .. but you know .. if what you doing makes sense to you ..that is all that is important.

@youroff
Copy link
Contributor

youroff commented Aug 5, 2016

Probably we need to take a closer look at scenarios, I thought they're for development. But even if we start using scenarios eventually, there might be other users who wanted to manage their mocks manually.

@danielspaniel
Copy link
Collaborator

danielspaniel commented Aug 5, 2016

I used to use scenarios only for development .. and then I started using those same scenarios I setup for dev in the tests .. because there was so much overlap .. and it just worked really well .. tests are clean and easy to read and factory guy is all in scenario.

Scenario is just a class with run method and preloaded factory guy methods. That is about all it is.

But benefit is if all data mocking/loading is there .. you can change out your data mocking/making without touching tests.

@youroff
Copy link
Contributor

youroff commented Aug 5, 2016

Sounds interesting, we'll look into that ;)

@ryedeer ryedeer deleted the deactivatemocks branch August 8, 2016 11:35
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.

3 participants