Landmines in Software, Landmines in Testing

Let's talk about landmines today, and what one looks like in software. Before we focus our attention on test cases, a land mine is something that doesn't happen often, takes you by surprise, and when it does, you lose an arm, a leg, or in our case, our minds at work. Minefields come in different shapes and sizes: sometimes they're heavily clustered, where each step is high risk, and others where it takes a lot of steps to step on a landmine. Are landmines not a metaphor for life? Maybe there's danger, but with enough diligence and good judgement, you can avoid stepping in it. Get too loosey goosey, too careless, too reckless, or (even just not diligent in) in life, you lose your girlfriend, your driver's license, a leg, or perhaps, eight hours on a three point story. There are indeed landmines in software, and landmines in testing Angular.

A Timeless Paradox On Many Dev Teams

You're a mid-level dev, fairly new to the team, looking at a pull request, somebody is mutating a fairly innocuous variable in JavaScript; you can vividly remember three months ago when you were on a two-hour debugging session because you missed the part where that variable's property was updated, a mutation, I hate mutations. You leave a comment saying, "Can we avoid mutating and stick to spread operator when possible?" Shortly after, you receive a private message on Slack, something to the effect of, "I don't get why this is a big deal, nothing's going to happen, I change the property's value, we do what works for us, Uncle Bob, or whoever the articles to the links your showing isn't gospel". You realize it's going to take a lot of time, earned trust, and pull request on your end with really clean code, maybe tackling some really complex requirements along the way, before people open up to suggested ideas — solid, time-tested best practices.

We can try to stay positive, but before that time comes: your team will raise PRs with mutations, and inevitably, a two-hour surprise falls into your lap, where that mutation slipped right under his or her nose, and the feature has an unexpected behavior because it wasn't accounted for in the later business logic. If you're new to this subject, you want to look into immutability and spread operator in JavaScript.

The Ultimate Land Mine for Spec Files

Ok, since you're either butthurt, or galvanized by me taking a jab at mutating, we're going to talk about mutating things in our test cases — kind of. Take a component with two display fields, fieldA and fieldB, which will get populated by the DataService on component initialization, along with a config object. We have method to submit the data from that form to a SubmitService, with a little validation in the mix.
I want to emphasize the the high likelihood of seeing this use case in a large application (and that you aren't likely to read every method called in methods you are testing), so I'm going to hide the declaration of the isNoNoRuleBroken and runConfigLogic methods from you, from the start.

export class DynamicFormComponent implements OnInit {
  fieldA: string = '';
  fieldB: string = '';
  submitMessage = '';
  config: { enforceTheNoNoRule: boolean } = { enforceTheNoNoRule: false };

  formatValidation = false;
  _isNoNoRuleBroken = false;

  constructor(
    private dataService: DataService,
    private submitService: SubmitService
  ) {}

  ngOnInit(): void {
    this.dataService.getFieldData()
      .pipe(
        take(1),
        switchMap(data => {
          this.fieldA = data.fieldA;
          this.fieldB = data.fieldB;
          return this.dataService.getValidationConfig().pipe(take(1));
        })
      )
      .subscribe(config => {
        this.config = config;
      });
  }

  private concatenateFieldsForPayload(): string {
    if (this.fieldA.length > 20 || this.fieldB.length > 20) {
      this.submitMessage = 'Payload is too long.';
      this.formatValidation = false;
      return '';
    }

    this.formatValidation = true;
    return `${this.fieldA} + ${this.fieldB}`;
  }

  submit(): void {
    const payload = this.concatenateFieldsForPayload();

    this.runConfigLogic();

    if (!this.formatValidation || this._isNoNoRuleBroken) {
      this.submitMessage = 'Validation failed: this is invalid.';
      return;
    }

    this.submitService.submitFormData(payload).pipe(take(1)).subscribe({
      next: () => {
        this.submitMessage = 'Form submitted successfully!';
      },
      error: () => {
        this.submitMessage = 'Form submission failed.';
      }
    });
  }

}

Here's our spec file, with some good ol' fashioned mock dependencies, with the behavior declared in the beforeEach callback.

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

  const mockDataService = {
    getValidationConfig: jest.fn(),
    getFieldData: jest.fn()
  };

  const mockSubmitService = {
    submitFormData: jest.fn()
  };

  beforeEach(async () => {
    mockDataService.getValidationConfig.mockReturnValue(
      of({ enforceTheNoNoRule: true })
    );
    mockDataService.getFieldData.mockReturnValue(
      of({ fieldA: 'testValue1', fieldB: 'testValue2' })
    );
    mockSubmitService.submitFormData.mockReturnValue(of(true));

    await TestBed.configureTestingModule({
      imports: [DynamicFormComponent],
      providers: [
        { provide: DataService, useValue: mockDataService },
        { provide: SubmitService, useValue: mockSubmitService }
      ]
    }).compileComponents();

    fixture = TestBed.createComponent(DynamicFormComponent);
    component = fixture.componentInstance;
    fixture.detectChanges();
  });

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

Let's Test this below submit function:

submit(): void {
    const payload = this.concatenateFieldsForPayload();

    this.runConfigLogic();

    if (!this.formatValidation || this._isNoNoRuleBroken) {
      this.submitMessage = 'Validation failed: this is invalid.';
      return;
    }

    this.submitService.submitFormData(payload).pipe(take(1)).subscribe({
      next: () => {
        this.submitMessage = 'Form submitted successfully!';
      },
      error: () => {
        this.submitMessage = 'Form submission failed.';
      }
    });
  }

Easy Peasy!

  it('should submit the fields loaded from the data service', () => {
    component.formatValidation = true;
    component._isNoNoRuleBroken = false;
    component.submit();
    fixture.detectChanges();
    expect(component.submitMessage).toBe('Form submitted successfully!');
  });

Except it doesn't pass...

What the heck?? If formatValidation is true (which we set to true), and _isNoNoRuleBroken is true (which we also set to true), it should work, right? Besides, we set the fields the appropriate length.

(And here's the part comes where you're like "Ok Cort, it was the hidden methods obviously")...and yes, time for the reveal. Here's the submit method again, followed by the hidden methods:

submit(): void {
    const payload = this.concatenateFieldsForPayload();

    this.runConfigLogic();

    if (!this.formatValidation || this._isNoNoRuleBroken) {
      this.submitMessage = 'Validation failed: this is invalid.';
      return;
    }

    this.submitService.submitFormData(payload).pipe(take(1)).subscribe({
      next: () => {
        this.submitMessage = 'Form submitted successfully!';
      },
      error: () => {
        this.submitMessage = 'Form submission failed.';
      }
    });
  }
  
 private runConfigLogic(): void {
  this._isNoNoRuleBroken = this.isNoNoRuleBroken();
 }
 
 private isNoNoRuleBroken(): boolean {
  return this.config.enforceTheNoNoRule && this.fieldB.includes('test');
 }

Notice we made our mock data service return testValue1 and testValueB in the spec file (which contain the word 'test')

mockDataService.getFieldData.mockReturnValue(
  of({
    fieldA: 'testValue1',
    fieldB: 'testValue2',
  })
);

Our "No no rule" all along was "anything with test in it is bad". Notice that if the DataService returned ({ enforceTheNoNoRule: true }, the config.enforceTheNoNoRule would be getting set to true in the subscribe block

ngOnInit(): void {
  this.dataService.getFieldData()
    .pipe(
      take(1),
      switchMap(data => {
        this.fieldA = data.fieldA;
        this.fieldB = data.fieldB;
        return this.dataService.getValidationConfig().pipe(take(1));
      })
    )
    .subscribe(config => {
      this.config = config;
    });
}

Which means when isNoNoRuleBroken and runConfigLogic are executed, _isNoNoRuleBroken will get set to true right underneath our noses in the submit method.

The Behavior vs State Paradigm

When we looked at

  submit(): void {
    const payload = this.concatenateFieldsForPayload();

    this.runConfigLogic();

    if(this.formatValidation === false || this._isNoNoRuleBroken) {
      this.submitMessage = 'Validation failed: this is invalid.';
      return;
    }
    
    this.submitService.submitFormData(payload).pipe(take(1)).subscribe({
      next: () => {
        this.submitMessage = 'Form submitted successfully!';
      },
      error: () => {
        this.submitMessage = 'Form submission failed.';
      }
    });
  }

Our first inclination was: "let's play with all the variables that are going to make this test pass", we tinkered with the state of the component — and in doing so, we forgot about the behavior of the component, or at least the parts outside of our immediate focust...hence the config setup on OnInit and the logic in runConfigLogic, (sneakily) came into play more than we expected.

it('should submit the fields loaded from the data service', () => {
  component.fieldA = 'testValue1';
  component.fieldB = 'testValue2';
  component.formatValidation = true;
  component._isNoNoRuleBroken = false;

  component.submit();
  fixture.detectChanges();

  expect(component.submitMessage).toBe('Form submitted successfully!');
});

How to focus on Behavior?

Remember how we used private to make you test functionality end-to-end, from event to outcome? What if we only relied on calling methods (unless there was no other way to cover it) to update all data in the component in preparation for calling a method to test. In doing so, you are forced to know the logic encapsulating all data updates? Now we focus on behavior first, and component state as a byproduct. We don't blow off behavior in favor of state for the easy win.

Ok, easy solution for us, in the spec file, make the mock data anything without test in it:

mockDataService.getFieldData.mockReturnValue(
  of({
    fieldA: 'mockValue1',
    fieldB: 'mockValue2',
  })
);

But notice, I made you change the behavior of the data service, which impacted OnInit, which made you look in the contents of OnInit, making you notice config.enforceTheNoNoRule comes into play in isNoNoRuleBroken.

What would that look like in a different application?

Suppose we had a component with a methods like

updateValues(fieldA: string, fieldB: string) {
    this.fieldA = fieldA;
    this.fieldB = fieldB;
  }

and

methodWhichUsesFieldAAndFieldB(): void {
  // Do stuff with fieldA and fieldB
}

Instead of writing a test like

it('should do stuff with fieldA and fieldB', () => {
  component.fieldA = 'testValue1';
  component.fieldB = 'testValue2';

  // Replace this with actual logic or assertions
  expect(/* stuff to be done with fieldA and fieldB */).toBe(true);
});

Instead do:

it('should do stuff with fieldA and fieldB', () => {
  component.updateValues('newValueA', 'newValueB');

  // Replace with actual assertion based on updated fieldA and fieldB
  expect(/* stuff to be done with fieldA and fieldB */).toBe(true);
});

Putting a Bow On This

Technically the term "Flow Control" was taken by Angular, to describe the new tools that allow for using @if, @else, @switch, in lieu of the traditional angular directives. However, even though HTML directives have nothing to do with what we talked about, I still like calling our testing approach "Flow Control", because we are using the "flow" of the component (by way of invoking methods) to both test methods and update the state of the component being tested to our liking. We're focusing on the flow of the component, and we are not relying on forcing state changes in the component variable through property assignments. This is going to highlight the behavior of the component leading up to, and within, the method you are testing, and consequently, you won’t waste time hunting for that mystery update that caused your test case to fail.