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

allow provider injection #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

chrisakers
Copy link

Hello @wardbell and contributors. I love how Bard allows injected services to be promoted to globals and makes the test relatively free from noise boilerplate instrumentation. But when I got around to adding tests for a provider I got a bit bummed that I had to add some boilerplate back in:

...
describe('personalizationConfig', function () {
    var personalizationConfigProvider;

    beforeEach(function () {
        bard.appModule('corp.personalization', function (_personalizationConfigProvider_) {
            personalizationConfigProvider = _personalizationConfigProvider_;
        });
        bard.inject(this, 'personalizationConfig');
    });

    describe('provider', function () {
        describe('initial state', function () {
...

So I wanted to see if it would be technically possible to allow:

...
describe('personalizationConfig', function () {
    beforeEach(function () {
        bard.appModule('corp.personalization');
        bard.inject(this, 'personalizationConfigProvider', 'personalizationConfig');
    });

    describe('provider', function () {
        describe('initial state', function () {
...

And was satisfied that this modified code performs as expected and demonstrates it's technically possible. However, much like John Hammond, I was so preoccupied with whether or not I could, I didn’t stop to think if I should.

Do you think this adds value? Does this violate some type of parity goal with standard angular.mock.module or inject? Thoughts?

@chrisakers
Copy link
Author

One note. This is just an exploratory PR to prove feasibility. I haven't considered all branches through inject so this PR should not be merged in the current state.

@wardbell
Copy link
Owner

Hi Chris. As I state in issue #45, I can't keep up any more. I spoke to John Papa and he thinks you'd be a motivated and capable successor. It's yours if you're willing to take it.

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