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(store): add @ngxs/store/internal/testing #1034

Merged
merged 8 commits into from
May 11, 2019
Merged

Conversation

splincode
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the new behavior?

own ability to manage testing more convenient

import { NgxsTestBed } from '@ngxs/store/testing';

describe('Zoo', () => {

  it('it toggles feed', (done) => {
    const { store } = NgxsTestBed.configureTestingState([ ZooState ]);

    store.dispatch(new FeedAnimals());
    store.selectOnce(state => state.zoo.feed).subscribe(feed => {
      expect(feed).toBe(true);
      done();
    });
  });

});

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@splincode splincode requested a review from markwhitfeld May 4, 2019 20:57
@markwhitfeld
Copy link
Member

@splincode What does this provide to the user that could not just be done simply through TestBed?

@splincode
Copy link
Member Author

splincode commented May 4, 2019

@markwhitfeld

  • First, at least we can get rid of the use of scattered setup functions in our code.

image

  • Secondly, using the usual TestBed users can not catch the method call ngxsAfterBootrstap in their state. Thanks to our wrapper, users can always test the full life cycle.

  • Third, we can now implement related issues:

#1007
#950
#843
#833
#482
#533

  • Improved testing
describe('Zoo', () => {
  let store: Store;

  beforeEach(() => {
     const describer = NgxsTestBed.configureTestingState(
            [ ZooState ],
            { developmentMode: false  },
           [ 
              FeatureModule,
              NgxsStoragePluginModule.forRoot() 
           ]
     );

     store = describer.store;
  })

  it('it toggles feed', (done) => {
     // use store
  });

});

@markwhitfeld
Copy link
Member

I believe that the creation of a @ngxs/store/testing package is essential and a high priority but I think that we need to take small steps to improve this. We need to create a coherent API that covers all testing needs.

IMO, the feature introduced by this particular PR is more focused on testing the ngxsAfterBootstrap in their state, not so much the general setup method. I also would not want to add a TestBed that does all the additional work for triggering the bootstrap but adds an additional cost to each test. Not ideal for unit testing.

I propose the following:

  • We create a @ngxs-labs/testing labs project so that we can quickly experiment, iterate and get feedback
  • We create helpers that directly solve each of the problems listed in the issues above
  • We elicit as much feeback/contribution as possible from the community
  • Then get it into the @ngxs/store/testing lib ASAP!

@markwhitfeld
Copy link
Member

PS. Down the line if we do end up creating our own NgxsTestBed.configureTestingState I would think it would be better to have a method signature that is compatible with the Angular TestBed.configureTestingState, but ours just adds additional configuration options and returns a describer that contains store and other objects convenient for ngxs testing.

@splincode
Copy link
Member Author

@markwhitfeld One of the main problems, why I don’t want to do this in a lab project, is because it’s not convenient to test on our internal tests. Having translated internal tests for our development we will be able to work out the API ourselves.

@markwhitfeld
Copy link
Member

Ok, could we then make these test helpers internal to our tests for now then?
We can move them to the sub package later

@splincode
Copy link
Member Author

splincode commented May 5, 2019

Sub package
@ngxs/store/experimental/testing

@uiii
Copy link

uiii commented May 5, 2019

Hi, see #1007 (comment) and #1007 (comment)

@splincode splincode changed the title feat(store): add @ngxs/store/testing test(store): add @ngxs/store/internal/testing May 5, 2019
@splincode
Copy link
Member Author

@markwhitfeld ping

@splincode
Copy link
Member Author

@markwhitfeld I add this PR for internal testing and fixes
#917

@splincode splincode merged commit 543b2bc into master May 11, 2019
@splincode splincode deleted the feat/testing branch May 11, 2019 23:40
@markwhitfeld
Copy link
Member

@splincode sorry I didn't get back to you. I have had a very busy week.
It's internal, so we can iterate in this a bit. Can always ship it to a labs package later if we choose to go that way as the first public api of this.

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