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

feat(stub-deps): ability to stub dependencies for ComponentTester #89

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

bigopon
Copy link
Member

@bigopon bigopon commented Feb 24, 2019

This adds ability to discard dependencies loading, per ComponentTester instance. What it does under the hood:

  • ensure all Aurelia instances get a loader of its own
  • patch loadTemplate method on those loaders
  • patch prototype method load of HtmlBehaviorResource to reset the view factory so it should always load fresh, instead of using the cache one.
  • reset prototype method load above on component disposal.

Supersedes #88
closes #88
closes #87
closes #20

@RomkeVdMeulen Can you try this branch and see if it works for your app? I didn't have very thorough test suit. Only accounted a few tests for:

  • html only dependencies
  • normal <require from="..."/>
  • <compose view-model="..."></compose>
  • <compose view="... .html"></compose>

Usage:

StageComponent
  .inView('some html')
  .ignoreDependencies('some/absolute/path/1', 'some/absolute/path/2')

Also added a static method inView for StageComponent class, it was a bit unnecessarily limited to have withResources() only, imo.

cc @fkleuver @zewa666 @EisenbergEffect

@bigopon
Copy link
Member Author

bigopon commented Feb 24, 2019

One thing I didn't test properly was using it together with requirejs/systemjs, so if there could be some help with testing it with those loaders, it would be great cc @davismj @huochunpeng

@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

I see you've updated a bunch of karma/webpack related settings. I guess they are solely affecting the test setup for the repo itself right? If I'm going to check it out with a Require.js based CLI project would I need to adjust anything specific except installing your PRs aurelia-testing branch?

@bigopon
Copy link
Member Author

bigopon commented Feb 25, 2019

@zewa666 Yes. I decided to sync this with our setup in other plugin repos 😛

@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

  • New project with latest CLI
  • TypeScript + RequireJS + Karma/Jasmine setup

in src folder:

// parent-component.html
<template>
  <require from="./child-component"></require>
  Parent <child-component message.bind="'Child'"></child-component>
</template>

// parent-component.ts
export class ParentComponent {
}

// child-component.html
<template>
  ${message}
</template>

// child-component.ts
import { bindable } from "aurelia-framework";

export class ChildComponent {
  @bindable() public message = '';
}

Test:

import { bootstrap } from 'aurelia-bootstrapper';
import { StageComponent } from "aurelia-testing";

describe('sample', () => {

  it('should render without and with child', async () => {
    let component;
    let subComponent;

    // First block, ignoring Child-component
    component = await StageComponent
      .withResources("parent-component")
      .inView('<parent-component></parent-component>')
      .ignoreDependencies('child-component');

    await component.create(bootstrap);
    subComponent = component.element as HTMLElement;
    expect(subComponent.textContent!.trim()).toEqual('Parent');
    expect(subComponent.textContent!.trim()).not.toContain('Child');

    await component.dispose();


    // Second block, keeping Child-component
    component = await StageComponent
    .withResources("parent-component")
    .inView('<parent-component></parent-component>')
    // .ignoreDependencies('child-component');
    
    await component.create(bootstrap);
    subComponent = component.element as HTMLElement;
    expect(subComponent.textContent!.trim()).toContain('Child');
    
    await component.dispose();
  });
});

The first one passes, the second one doesn't. It seems like the recreation of the StageComponent still remembers that the ignoreDependencies as of before is active. Commenting out the first block makes the second part of the test pass.

@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

Another sideffect is if i do this

// OLD
.withResources("parent-component")

// NEW
.withResources(["parent-component", "child-component"])

the ignoreDependencies doesn't work at all. Guess this is not intended right?

Copy link
Contributor

@RomkeVdMeulen RomkeVdMeulen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried this out with our SystemJS test suite and it works great.

I have a couple of suggestions:

src/component-tester.ts Outdated Show resolved Hide resolved
src/component-tester.ts Outdated Show resolved Hide resolved
RomkeVdMeulen and others added 2 commits February 25, 2019 23:26
Thanks @RomkeVdMeulen

Co-Authored-By: bigopon <9994529+bigopon@users.noreply.github.com>
Co-Authored-By: bigopon <9994529+bigopon@users.noreply.github.com>
@bigopon
Copy link
Member Author

bigopon commented Feb 25, 2019

@RomkeVdMeulen Thanks for the suggestion, they are very nice. It would be great if you could help on documentation of this feature too.

@RomkeVdMeulen
Copy link
Contributor

Sure thing. I'll go get started.

RomkeVdMeulen added a commit to RomkeVdMeulen/documentation that referenced this pull request Feb 25, 2019
This describes a new feature of aurelia-testing introduced by
aurelia/testing#89
@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

@RomkeVdMeulen could you share the test you were performing with SystemJS? Wonders me why it failed with requirejs as depicted above.

@RomkeVdMeulen
Copy link
Contributor

@bigopon Thanks for including my suggestion to make the calls cumulative, but as it stands it won't work anymore: this.stubbed is never initialized.

@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

just as another side-note ... for RequireJS based projects ignoreDependencies accepts the relative path to src. So my sample as above is just .ignoreDependencies('child-component'); vs .ignoreDependencies('src/child-component');

@RomkeVdMeulen
Copy link
Contributor

RomkeVdMeulen commented Feb 25, 2019

@zewa666 Our project is huge so that won't be practical, but what I'm doing is basically no different from the tests in template-dependency-stub.spec.ts.

Edit: our project uses an alias for our src dir, rather than relative paths. Maybe that influences things?

@zewa666
Copy link
Member

zewa666 commented Feb 25, 2019

So attached please find a minimalistic example I've created with the latest CLI and RequireJS. Maybe you can spot the difference @RomkeVdMeulen Archive.zip

@RomkeVdMeulen
Copy link
Contributor

RomkeVdMeulen commented Feb 25, 2019

@bigopon Could you take a look at @zewa666's sample code? It seems the second test, without ignored dependencies, isn't working well. I'm not sure why. Looks like the dependency is loaded, but something else is going wrong preventing the child component from rendering properly. If I dump the rendered HTML at the end of the test, it looks like this:

<parent-component class="au-target" au-target-id="4">

  Parent <child-component message.bind="'Child'" class="au-target" au-target-id="1"></child-component>
</parent-component>

From the au-target-id is seems the child-component is loaded, but not rendered properly.

EDIT: Nevermind, I seem to be on the wrong path. If you switch the two tests, you get

<parent-component class="au-target" au-target-id="4">
  
  Parent <child-component message.bind="'Child'" class="au-target" au-target-id="2">
  Child                                            
</child-component>
</parent-component>

when child-component should be ignored.

@bigopon
Copy link
Member Author

bigopon commented Feb 26, 2019

@RomkeVdMeulen @zewa666 Can you help change the configuration of dependencies to be ignored to absolute path? It's what loader will resolve to eventually.

@zewa666
Copy link
Member

zewa666 commented Feb 26, 2019

I'm not sure this has anything to do with the absolute path though @bigopon. It works the first time and is able to match the dependency. It's just on the second try it screws up.

@@ -183,3 +184,39 @@ describe('Template dependency stubbing', () => {
});
});
});

fdescribe('sample', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is debug code?

@RomkeVdMeulen
Copy link
Contributor

Nice work!

@bigopon
Copy link
Member Author

bigopon commented Feb 26, 2019

I'm trying to add some base tests for requirejs to make sure things run smoothly. For the same setup, i didn't get any issues with Webpack.

@RomkeVdMeulen
Copy link
Contributor

@bigopon I can't build the latest version. I'm getting this error:

node_modules/@types/node/index.d.ts(140,13): error TS2451: Cannot redeclare block-scoped variable 'require'.                                                                                                   
[5] test-requirejs/setup.ts(9,15): error TS2451: Cannot redeclare block-scoped variable 'require'.

Is the current version incomplete? Or were these errors not supposed to occur?

@RomkeVdMeulen
Copy link
Contributor

I got the build back by replacing declare const require in test-requirejs/setup.ts with:

const req: any = require;

@RomkeVdMeulen
Copy link
Contributor

Also: the main property in package.json had to be updated to dist/commonjs/src/aurelia-testing.js to get this to work.

Running @zewa666's sample against the latest version still doesn't work as it should: the dependencies are still ignored when the component tester is re-initialized without ignored dependencies.

@bigopon
Copy link
Member Author

bigopon commented Mar 1, 2019

@RomkeVdMeulen I'm still trying to figure out what went wrong in requirejs setup. It's not any easier to debug than webpack. Also in requirejs things are not resolved to their absolute url like webpack, so it's another layer that needs to be accounted.

@bigopon
Copy link
Member Author

bigopon commented Mar 31, 2019

There are a number of cache layers in combination of requirejs + default loader + templating. I have not been able to successfully work out how to invalidate cache of all layers for each test. I've reverted the latest code so that if anyone who uses webpack can easily use my branch and build it.

@RomkeVdMeulen
Copy link
Contributor

@bigopon Is there perhaps someone we could pull in on this who is more in-depth on the caching layers? I also wonder wether this will work for Aurelia 2.

@bigopon
Copy link
Member Author

bigopon commented Dec 3, 2019

V2 will not, if never, suffer any issue like this, that im pretty confident about. @fkleuver has done an amazing job to ensure apps are properly containerized. How is my branch going for you?

@RomkeVdMeulen
Copy link
Contributor

RomkeVdMeulen commented Dec 4, 2019

Working fine with System loader.

\EDIT: And using absolute module paths (with an alias in my case). Since that seems to be an issue.

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.

Feature request: Mocking/stubbing sub-views
3 participants