Angular Code Coverage shows function not being covered

734 views Asked by At
  reportsRes: ReportResponse[] = null;
 
  constructor(private userReportService: UserReportsService) {
   }
  ngOnInit(): void {
    this.getUserReports2();
  }
 
  private getUserReports2(): void {
    this.userReportService.getMyReports3().subscribe((resp: ReportResponse[]) => {
      this.reportsRes = resp;
    }, (error: any) => {
      console.log("error: ", error);
    });
  }

Above is all I have in my component. I cannot understand why it shows that I am not covering my getUserReports2 function:

enter image description here

If the image does not pull up, then from the test report, it shows the below bolded code in red:

ngOnInit()***: void {***
    this.ge***tU***serReports2();
  }

Below are few of the different ways I have tried to test, but to no avail. It never gets me to 100% test coverage:

it('should create', () => {
    component.ngOnInit();
    expect(component).toBeTruthy();
  });

  it('should getUserReports', () => {
    const reportService: UserReportsService = TestBed.inject(UserReportsService);
    let mockData: ReportResponse[] = [{
      id: "",
      frFile: "",
      reportData: {
        aor: "",
        aorName: "",
        dco: "",
        cbpHoldFacility: false,
        dcoName: "",
        detained: false,
        detention: "",
        detentionName: "",
        endDate: "",
        iceHoldFacility: false,
        id: "",
        nonDetained: false,
        rawDataOnly: false,
        releaseReasonsCds: null,
        lastIntactLocation: null,
        reportType: "PDF",
        reunificationLocation: null,
        startDate: "",
        submittedOn: ""
      },
      status: Constant.FINISHED_STATUS
    }];
    const reportServiceSpy = spyOn(reportService, 'getMyReports3').and.returnValue(of(mockData));
    // component.getUserReports2();
    // expect(component.reportsRes.length).toBe(0);
    console.log("after component.getUserReports()");
    // expect(reportServiceSpy).toHaveBeenCalledTimes(1);
  });

  it('should getUserReports throwError', (done: DoneFn) => {
    const reportService: UserReportsService = TestBed.inject(UserReportsService);
    const reportServiceSpy = spyOn(reportService, 'getMyReports3').and.callThrough();
    expect(component.reportsRes).toBeNull();
    done();
  });
1

There are 1 answers

0
SirOneOfMany On

Ok I want to add an answer to your question. The problem with your approach are usually the heavy use of lambdas. They get detected as untested functions.

By using .subscribe() you provide an imperative approach. To make this more testable, you could write an observer in another file and then test its functions:

https://rxjs.dev/guide/observer or http://reactivex.io/rxjs/class/es6/MiscJSDoc.js~ObserverDoc.html

So in your case:

export class YourCustomObserver implements Observer<ResponseTypeOfYourCall> {
   constructor(private injectedPropertyThatShouldBeChanged) {}    
   
   error(error: any): void {
       console.error(error)
   }

   next(value: ResponseTypeOfYourCall) {
      this.injectedPropertyThatShouldBeChanged = value;
   }

   complete(): void {
      return;
   }
}

Then you should be able to use it in your quirky component:

 private getUserReports2(): void {
    this.userReportService.getMyReports3().subscribe(new YourCustomObserver(this.reportsRes));
  }

BUT

And this is a fat BUT: You will not need all of this when you just use the async pipe.

Now let me try to convince you to use this conventioned practice:

1. Reduction of unnecessary state

Unless you set your component to ChangeDetectionStrategy.OnPush, every property is being observed by the change detector. This has a performance impact on your application. And JavaScript - what angular basically is - runs within one single thread. So state reduction increases runtime performance.

2. Mutable state and a tale of side effects

Mutable state is imho a general anti pattern in the programming world. Why? Because it introduces unforseen side effects. I saw a lot of code where changing one single state keeping property had an effect on the whole component tree. One property might be another components input. And components with changeDetectionStrategy.OnPush will not change at all when you just mutate the state. In your case the subscription will change the properties reference and therefore it might be working. So generally in angular it is way better to use immutable, yet reactive state changes.

3. It bloats your code base

You see in your example that you had to introduce a lifecycle hook that calls a private method which subscribes to an observable resolving the backend call which then changes a property to the result of your subscription. Sounds like a Kennedy Assassination, doesn't it? Why not just:

readonly userReports$ = this.userReportService.getMyReports3();
constructor(private readonly userReportService: UserReportsService) {}

And then just:

[someProp]="userReports$ | async"

4. It is not reactive

Your approach takes out the reactive part and introduces an imperative approach with mutual state on an asynchronous operation. Keep it reactive and make your code declarative or do not use observables at all.

5. Subscriptions need management

Unmanaged subscriptions, especially the long running ones, will result in a memory leak. So you need a way to unsubscribe from them. If done by calling unsubscribe() or via the take*-Operators is up to you, but in all cases you need subscription management. Http calls usually complete so they do not need that big attention but for example Subjects are usually sources for memory leaks.

  1. Separating data from state

Data and component state are not the same thing. Yes data can be part of the component state but the state is not limited to this data. Mixing both concerns will lead to painful bugs and bad side effects. Therefore both concerns should be seperated: Keeping the stream mutation functions is state, the transformed result is data. So instead of keeping the data as a property in your state you should just keep the Observable stream as state and everything that is tightly coupled to components behavior within the state. Sometimes this is being argued and the most used argument is a disabled state. But where does this disabled state come from? Is it a result from some data changes? Then it is part of the data, or better the transformation of the data expressed as a boolean. Is it coming from outside the component? Then it is definetly data, lets say from the forms API. In a reactive component that uses rxjs, state is expressed as functions that do transformations on streams. So keeping this streams is state, not the data that they are emitting.

To your statement that you do not use the async pipe because of "personal preference" is hopefully either just a bad joke or you can explain it rationally. But considering the points from above I cannot see any good explanation why someone should have the personal preference to not use the technical better approach unless this personal preference is resulting from missing skill or knowledge.