wangzixi-diablo / Spartacus-learning

record questions raised during my study
0 stars 1 forks source link

observation about synchronization issue between button.disabled and component.mode.controls['deliveryModeId'].invalid #4

Closed wangzixi-diablo closed 3 years ago

wangzixi-diablo commented 3 years ago

background

file location: projects\storefrontlib\src\cms-components\checkout\components\delivery-mode\delivery-mode.component.html

we are now talking about property binding of "continue" button's disabled property.

Expected behavior

when component property deliveryModelnvalid = true, continue button's disabled property will be set as true as well, so button is disabled, and vice versa.

clipboard1

Below is the getter implementation of deliveryModeInvalid in Component:

clipboard2

so if we set this.mode.controls['deliveryModeId'] to '' or null, deliveryModeInvalid will equal to true.

Jerry's observation in unit test

if we change the value of this.mode.controls['deliveryModeId'] TWICE in the same test spec, [disabled] and deliveryModeInvalid will LOSE synchronization, even fixture.detectChanges is called manually.

See my test below.

Paste the following source code to replace your delivery-mode.component.spec.ts and launch it:

import { Component } from '@angular/core';
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { ReactiveFormsModule } from '@angular/forms';
import { ActivatedRoute } from '@angular/router';
import {
  CheckoutDeliveryService,
  DeliveryMode,
  I18nTestingModule,
} from '@spartacus/core';
import { Observable, of } from 'rxjs';
import { CheckoutConfigService } from '../../services/checkout-config.service';
import { CheckoutStepService } from '../../services/checkout-step.service';
import { DeliveryModeComponent } from './delivery-mode.component';
import createSpy = jasmine.createSpy;
import { LoaderState } from '../../../../../../core/src/state/utils/loader';
import { By } from '@angular/platform-browser';

@Component({
  selector: 'cx-spinner',
  template: '',
})
class MockSpinnerComponent {}

class MockCheckoutDeliveryService {
  loadSupportedDeliveryModes = createSpy();
  setDeliveryMode = createSpy();
  getSupportedDeliveryModes(): Observable<DeliveryMode[]> {
    return of();
  }
  getSelectedDeliveryMode(): Observable<DeliveryMode> {
    return of();
  }
  getLoadSupportedDeliveryModeProcess(): Observable<LoaderState<void>> {
    return of();
  }
}

class MockCheckoutConfigService {
  getPreferredDeliveryMode(): string {
    return '';
  }
}

class MockCheckoutStepService {
  next = createSpy();
  back = createSpy();
  getBackBntText(): string {
    return 'common.back';
  }
}

const mockActivatedRoute = {
  snapshot: {
    url: ['checkout', 'delivery-mode'],
  },
};

describe('DeliveryModeComponent', () => {
  let component: DeliveryModeComponent;
  let fixture: ComponentFixture<DeliveryModeComponent>;

  beforeEach(async(() => {
    TestBed.configureTestingModule({
      imports: [ReactiveFormsModule, I18nTestingModule],
      declarations: [DeliveryModeComponent, MockSpinnerComponent],
      providers: [
        {
          provide: CheckoutDeliveryService,
          useClass: MockCheckoutDeliveryService,
        },
        { provide: CheckoutStepService, useClass: MockCheckoutStepService },
        { provide: CheckoutConfigService, useClass: MockCheckoutConfigService },
        { provide: ActivatedRoute, useValue: mockActivatedRoute },
      ],
    }).compileComponents();
  }));

  beforeEach(() => {
    fixture = TestBed.createComponent(DeliveryModeComponent);
    component = fixture.componentInstance;
    console.log('----------- a new test comes ------------------');
  });

  describe('continue button', () =>{

    const getContinueBtn = () => fixture.debugElement.query(By.css('.cx-checkout-btns .btn-primary'));
    const setDeliveryModeId = (value: string) =>  
      component.mode.controls['deliveryModeId'].setValue(value);

    function setDeliveryModeIdNull(){
      setDeliveryModeId(null);
      fixture.detectChanges();
      trace(null);
    }
    function setDeliveryModeIdValid(){
      setDeliveryModeId('a');
      fixture.detectChanges();
      trace('a');
    }

    function trace(id){
      const button = getContinueBtn();
      console.log('************** Delivery Mode id is set as: ' + id + '**************');
      console.log('Flag component.deliveryModeInvalid: ' + 
      component.deliveryModeInvalid + ' button.disabled: ' + button.nativeElement.disabled );
    }

    function valid_then_null(){
      setDeliveryModeIdValid();

      setDeliveryModeIdNull();
    }

    function null_then_valid(){
      setDeliveryModeIdNull();

      setDeliveryModeIdValid();
    }

    it('first valid then null', () => {
      valid_then_null();
      expect(component).toBeTruthy();
    });

    it('null then valid', () => {
      null_then_valid();
      expect(component).toBeTruthy();
    });

    it('only valid', () => {
      setDeliveryModeIdValid();
      expect(component).toBeTruthy();
    });

    it('only null', () => {
      setDeliveryModeIdNull();
      expect(component).toBeTruthy();
    });
  });
});

In this unit test file I construct four test specs:

(1) set delivery mode id to a valid value first, then set null; (2) set delivery mode id to null first, then set a valid value to it; (3) only set a valid value; (4) only set null;

In each test spec, once I manupulate the value of this.mode.controls['deliveryModeId'], then use console.log to display the following pair of values:

In theory the two must always be equal.

Test result

clipboard3 clipboard4

Conclusion

Avoid change delivery mode id TWICE in a single test spec. The case to set id as A in beforeEach and set id as B in a test spec SHOULD also be considered as TWICE, so the two value will lose synchronization as well.

wangzixi-diablo commented 3 years ago

for issue: https://github.com/SAP/spartacus/issues/9294

Platonn commented 3 years ago

There is known issue in Angular testing framework. The fixture.detectChanges() works only when called for the first time in spec if your component has cofigured ChangeDetectionStrategy.OnPush. (See angular issue: https://github.com/angular/angular/issues/12313)

You can fix it, by amending the ChangeDetectionStrategy for your testing component, using TestBed.overrideComponent. See for example: https://github.com/SAP/spartacus/blob/develop/projects/storefrontlib/src/shared/components/table/table.component.spec.ts#L42-L44

Reference article: https://medium.com/@juliapassynkova/how-to-test-onpush-components-c9b39871fe1e