Add address form tests #114
Conversation
75dd7d0
to
468fb5a
Compare
|
Where is a test spec for |
|
|
||
| describe("when view model initialized") { | ||
| it("should have variables with correct initial values") { | ||
| expect(viewModel.address).to(beNil()) |
There was a problem hiding this comment.
You also need to check all rx variables that initialized with empty string.
| it("should return correct contry names") { | ||
| viewModel.namesOfCountries | ||
| .subscribe(onNext: { names in | ||
| expect(names) == ["Country1", "Country2"] |
There was a problem hiding this comment.
Use toEventually method to compare values in subscribe closures if this subject changes after subscription.
| } | ||
|
|
||
| context("if states not found") { | ||
| it("should return empty array") { |
There was a problem hiding this comment.
You also need to check stateText.value = "" in this test case.
|
|
||
| viewModel.filledAddress | ||
| .subscribe(onNext: { filledAddress in | ||
| expect(filledAddress.country) == address.country |
There was a problem hiding this comment.
I think we need to cover all changes so test all variables that can be changed by updateFields method call.
| @testable import ShopApp | ||
|
|
||
| class AddressFormViewModelSpec: QuickSpec { | ||
| override func spec() { |
There was a problem hiding this comment.
You also need to test isAddressValid and submitTapped properties.
|
|
||
| describe("when view loaded") { | ||
| beforeEach { | ||
| viewController = UIStoryboard(name: StoryboardNames.account, bundle: nil).instantiateViewController(withIdentifier: ControllerIdentifiers.addressForm) as! AddressFormViewController |
There was a problem hiding this comment.
Why are you duplicate initialization of controller and outlets few times? We can do it one time in beforeEach closure before first describe closure with test cases.
| viewModelMock = AddressFormViewModelMock(countriesUseCase: addressFormUseCaseMock) | ||
| viewController.viewModel = viewModelMock | ||
|
|
||
| countryPicker = self.findView(withAccessibilityLabel: "addressFormCountryPicker", in: viewController.view) as! BasePicker |
There was a problem hiding this comment.
Can we avoid this init in each test case? We already do it in previous beforeEach method
| viewModelMock.makeSubmitButtonEnabled() | ||
|
|
||
| viewController.viewModel.filledAddress | ||
| .subscribe(onNext: { address in |
There was a problem hiding this comment.
Can we avoid subscription in test case and test real binding between view controller and view model?
|
|
||
| it("should return correct contry names") { | ||
| viewModel.namesOfCountries | ||
| .subscribe(onNext: { names in |
There was a problem hiding this comment.
Can we avoid all subscriptions in tests?
There was a problem hiding this comment.
It's PublishSubject, so we need to subscription in this case
|
|
||
| context("if it doesn't have any symbols in each text field view") { | ||
| it("needs to disable submit button") { | ||
| expect(submitButton.isEnabled) == false |
There was a problem hiding this comment.
Add viewModelMock.makeSubmitButtonDosabled() before expect, it will be more correct, especially in case when this test case will be execute after previous one.
| }) | ||
| .disposed(by: disposeBag) | ||
|
|
||
| viewModelMock.submitTapped.onNext() |
There was a problem hiding this comment.
It should be submit button action, not view model observer event. With whit changes you automatically test binding between button and view model.
There was a problem hiding this comment.
Also test that view end editing after button pressed.
| @testable import ShopApp | ||
|
|
||
| class AddressFormViewControllerSpec: QuickSpec { | ||
| override func spec() { |
There was a problem hiding this comment.
You also need to test:
- binding of text fields/pickers with view model's variables
- pickers' item select actions (if it possible)
- pickers' custom data and text that setup with data from view model's subscription
647841d
to
f0051fa
Compare
| } | ||
|
|
||
| it("should select country correctly") { | ||
| countryPicker.pickerView.selectRow(0, inComponent: 0, animated: false) |
There was a problem hiding this comment.
Don't you need to call viewModelMock.makeNameOfCountries() before testing?
| }) | ||
| .disposed(by: disposeBag) | ||
|
|
||
| viewModelMock.makeNameOfCountries() |
There was a problem hiding this comment.
Make arguments for this method and pass values which need to be emitted from test. It will be more readable.
| }) | ||
| .disposed(by: disposeBag) | ||
|
|
||
| viewModelMock.makeNameOfStates() |
There was a problem hiding this comment.
Make arguments for this method and pass values which need to be emitted from test. It will be more readable.
| } | ||
| } | ||
|
|
||
| func initData(addressNeeded: Bool) { |
There was a problem hiding this comment.
Agree, but we have error 'Attribute 'private' can only be used in a non-local scope'. If we want to move this method out of 'spec()' function, it also need to move all properties (like viewController, viewModel, etc)
|
|
||
| viewModel.namesOfStates | ||
| .subscribe(onNext: { names in | ||
| expect(names.isEmpty) == false |
There was a problem hiding this comment.
Can we check here if nameOfStates returns what we expected?
|
|
||
| context("if address not filled") { | ||
| beforeEach { | ||
| address = Address() |
There was a problem hiding this comment.
You can name this field like validAddress or invalidAddress for more readable - not issue, but can improve tests in future
4976a77
to
cc6505d
Compare
| } | ||
|
|
||
| it("should have correct view model namesOfCountries binding") { | ||
| viewController.viewModel.namesOfCountries |
There was a problem hiding this comment.
Do you really need subscription here?
| } | ||
|
|
||
| it("should have correct view model namesOfStates binding") { | ||
| viewController.viewModel.namesOfStates |
There was a problem hiding this comment.
Do you really need subscription here?
Add address form tests