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

Feature request: Mocking/stubbing sub-views #20

Open
yacuzo opened this issue Jun 30, 2016 · 24 comments · May be fixed by #89
Open

Feature request: Mocking/stubbing sub-views #20

yacuzo opened this issue Jun 30, 2016 · 24 comments · May be fixed by #89

Comments

@yacuzo
Copy link

yacuzo commented Jun 30, 2016

Hi,

I know this is heavily a work-in-progress library, but I also know you want feedback :)

Having used the ComponentTester for a few components we find the need for mocking/stubbing out sub-views which are imported through the html-require tag.

Prehaps a bit of background is helpful:
We have a separate suit of tests we call binding-tests. They are a form of regression-tests to avoid the problem of renaming view model properies and forgetting to rename the bindings in the HTML.
We want these as unit-tests so that we can get feedback asap.

This creates a problem where we have a component foo:

export class Foo {
  fooData;
  get processedFoo() {
    return processFoo();
  }
}

With html:

<template>
  <require from="some/path/bar" />
  <input value.bind="fooData" />

  <bar some-data.bind="processedFoo"></bar>
</template>

We want to check the binding for bar's 'some-data', but we do not want to parse the possibly complex output of bar in order to check that. Bar's output should not be a part of the test for Foo.

Is there a way to do this currently? Have you considered a use-case like this?

@EisenbergEffect
Copy link
Contributor

I'd say, the best way to get something like this in would be to submit a PR with the proposed API.

However, for this specific scenario, it seems like you would just want to gain access to the view-model for bar and check it's property. That is possible, though a nicer API could be made available.

@yacuzo
Copy link
Author

yacuzo commented Aug 2, 2016

I dont want to check the property on the model. I want to check that my binding-expression in foo's template corresponds with something on the model. As I see it the best way to do this would be to mock out the bar-component, and check that the mock renders something other than undefined.

I'm not sure what the API for mocking out 'bar' would be. The ComponentTester.WithResources needs a way to override the require-tag in the template.

@suneelv
Copy link

suneelv commented Jul 3, 2017

We have very complex custom elements which require many other custom elements. To test any one of the complex custom element we had to mock the dependencies of all the other required custom elements which is not ideal.
As @yacuzo already mentioned, if the component tester provides a way to only render the top level custom element, then it would greatly reduce our element setup for testing.

@RomkeVdMeulen
Copy link
Contributor

Exactly this. For example, say I have a component Mycomponent:

<template>
    <require from="./mysubcomponent"></require>

    <mysubcomponent if.bind="myData" data.bind="myData"></mysubcomponent>
</template>

I've already written a separate test for Mysubcomponent, mocking out services and the like. I don't want to have to mock out those same services again in my test of Mycomponent just to test whether the if.bind works as expected.

Is there some workaround to mock out mysubcomponent during the test of mycomponent?

@zewa666
Copy link
Member

zewa666 commented Feb 13, 2018

This pretty much sounds like a shallow rendering of the top level component and ditching childs. So essentially If we'd unregister the child elements they should be rendered as normal html.

@RomkeVdMeulen
Copy link
Contributor

@zewa666 Any way to do that manually with the current API?

@zewa666
Copy link
Member

zewa666 commented Feb 15, 2018

We're currently looking into it to find the manual way. Will post back here and then go for a nicer overall API.

@RomkeVdMeulen
Copy link
Contributor

@zewa666 Great! Thank you all for the prompt support.

@RomkeVdMeulen
Copy link
Contributor

Any progress on this? I've been stubbing support for services used by subcomponents, and it's starting to become a bit tiresome.

@bigopon
Copy link
Member

bigopon commented Mar 11, 2018

We can do this, via hooking to afterCompile, but how would the API would be like:

  .withResources('module1', {
    exclude: {
      elements: ['name1', 'name2']
    }
  }) 

or

  .withResources('...')
  .discardElements('name1', 'name2')

?

@zewa666
Copy link
Member

zewa666 commented Mar 11, 2018

How about .shallow() and exclude all internally?

@bigopon
Copy link
Member

bigopon commented Mar 12, 2018

What about the <require/> statement, do we skip it entirely ? as when loading a module like foo.js to get custom element <foo/>, we might have additional resource to load ?

@RomkeVdMeulen
Copy link
Contributor

It might be good to have the option of excluding some but not all subcomponents, in addition to .shallow()

@zewa666 Have you found a current workaround for this problem?

@bigopon
Copy link
Member

bigopon commented May 25, 2018

I think it's quite hard to shallow elements by name, and I think it's the loading sub module behavior that we don't want, not the rendering the elements.

For example:

module A --requires--> module B --requires--> module C

When saying the test should ignore b, and c, does it implies (1) do not touch module B and module C, or (2) load module B, and load module C but do not do anything after that ? For me, (1) looks more desirable. Solution for (1) is also easier to create, we only need to patch the loader loadAllModules(...) (as it is called by ViewEngine.prototype.importViewResources) with a set of ignored module. I'm not sure about (2)

@RomkeVdMeulen
Copy link
Contributor

In theory the module files for subcomponents shouldn't even be loaded. However, modules with custom binding behaviors or value converters do need to be loaded for a proper test. I'm not sure you can distinguish them a priori, so some blacklisting or whitelisting API will probably be necessary.

@RomkeVdMeulen
Copy link
Contributor

There doesn't seem to be any progress on this. Is there anything I can do to help further lock down the API for this?

@bigopon
Copy link
Member

bigopon commented Feb 12, 2019

@RomkeVdMeulen If this is only stubbing dependencies from <require> element, then couple of override methods related to dependency management in TemplateRegistryEntry will work, as once demonstrated by lazy global resources from @huochunpeng

For static view strategy, either via static $view field/method or @view decorator, we just need to override method loadViewFactory of it to adjust the dependencies before loading, and return it back to normal after loading.

@RomkeVdMeulen
Copy link
Contributor

@bigopon I'd like to try this out on our tests, see if it does what we need. I'm not sure where to start though. Is there a way for me to patch something locally to try out your ideas?

@bigopon
Copy link
Member

bigopon commented Feb 14, 2019

Here is the answer from @huochunpeng in a discourse thread https://discourse.aurelia.io/t/lazy-load-global-custom-elements/1627/8

It's basically an override on TemplateRegistryEntry.prototype.template setter in aurelia-loader module to automatically add dependency based on a globally available path, locally to that template.

We can apply the same technique, to automatically remove all <require/> that match stubs.

For static view strategy, either via static $view field/method or @view decorator, we just need to override method loadViewFactory of it to adjust the dependencies before loading, and return it back to normal after loading.

This is about our new feature: static view. Normally dependencies is defined like following:

@view({
  template: '<template></template>',
  dependencies: () => [SomeClass, import('some/module')],
  // or
  dependencies: [SomeClass, import('some/module')]
})
export class MyElement {
  ...
}

or

export class MyElement {
  static $view = {
    template: '<template></template>',
    dependencies: () => [SomeClass, import('some/module')],
    // or
    dependencies: [SomeClass, import('some/module')]
  }
}

This is harder to stub, and I think it will require some patching on StaticViewStrategy.prototype.loadViewFactory export of aurelia-templating

StaticViewStrategy.prototype.loadViewFactory = async function() {
  const dependencies = await Promise.all(this.dependencies);
  // now what to do with dependencies ???
  //  they are all classes, or module objects
}

I'm not sure about this.

cc @zewa666 @EisenbergEffect

@EisenbergEffect
Copy link
Contributor

@RomkeVdMeulen If this works for your project, perhaps we can extract your code into a helper method that we add to this testing library. We can start with the require-based scenario first and then support static views in a second iteration. If most people are using require-based views then that lets us get the bulk of people a solution sooner.

@RomkeVdMeulen
Copy link
Contributor

@bigopon Thanks! I got it to work with our <require> based setup. I adjusted @huochunpeng's approach a bit: there's no need to traverse the whole template's tree, we can simply filter out the unwanted dependencies afterward:

import {TemplateDependency, TemplateRegistryEntry} from "aurelia-loader";

export const STUBBED_DEPENDENCIES: string[] = [];

const templateDescriptor = Object.getOwnPropertyDescriptor(TemplateRegistryEntry.prototype, "template")!;
const oldGet = templateDescriptor.get!;
const oldSet = templateDescriptor.set!;
Object.defineProperty(TemplateRegistryEntry.prototype, "template", {
  get: oldGet,
  set(value: HTMLTemplateElement) {
    oldSet.call(this, value);
    if (STUBBED_DEPENDENCIES.length > 0) {
      this.dependencies = this.dependencies
          .filter((d: TemplateDependency) => !STUBBED_DEPENDENCIES.includes(d.src));
    }
  },
});

Example use (made this up without checking):

describe("Parent component", () => {

  before(() => STUBBED_DEPENDENCIES.push("src/components/child-component/child-component"));
  after(() => STUBBED_DEPENDENCIES.splice(0));

  it("doesn't load dependencies you don't want it to", async () => {
    const component = await StageComponent
        .withResources('parent-component')
        .inView('<parent-component></parent-component>')
        .create(bootstrap);

    expect(component.innerHTML).to.equal("<child-component></child-component>"
        + "<other-child>This does get loaded</other-child>");
  });
});

@bigopon
Copy link
Member

bigopon commented Feb 15, 2019

@RomkeVdMeulen Yes this way is better, when it's only about dependencies removal. Would you accept the invitation of helping folks out please 😁

@EisenbergEffect
Copy link
Contributor

We'd love a pull request @RomkeVdMeulen 😄

@RomkeVdMeulen
Copy link
Contributor

I went home for today. I'll take a look at it monday.

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

Successfully merging a pull request may close this issue.

6 participants