wokwi / avr8js

Arduino (8-bit AVR) simulator, written in JavaScript and runs in the browser / Node.js
https://blog.wokwi.com/avr8js-simulate-arduino-in-javascript/
MIT License
461 stars 73 forks source link

Watchdog unit test seems to work not correctly #115

Closed Dudeplayz closed 2 years ago

Dudeplayz commented 2 years ago

While I was working at the peripherals, I discovered that the WDR command does not need to be run in order to finish all the watchdog unit tests. But one unit test is written to exactly test this.

https://github.com/wokwi/avr8js/blob/e76663c35d89f6ce719f5565eae09aef2366f5f6/src/peripherals/watchdog.spec.ts#L93-L130

If you comment out https://github.com/wokwi/avr8js/blob/e76663c35d89f6ce719f5565eae09aef2366f5f6/src/peripherals/watchdog.ts#L63 it is still successful, which should not be the case. So the question is, is the unit test written wrong or is there another reason? Locally I have also enhanced the unit test with a spy on the callback:

    const callbackSpy = jest.spyOn(cpu, 'onWatchdogReset');

    // Setup: enable watchdog timer
    runner.runInstructions(4);
    expect(watchdog.enabled).toBe(true);
    expect(callbackSpy).not.toHaveBeenCalled();

    // Now we skip 8ms. Watchdog shouldn't fire, yet
    cpu.cycles += 16000 * 8;
    runner.runInstructions(1);
    expect(callbackSpy).toHaveBeenCalled();
urish commented 2 years ago

is the unit test written wrong or is there another reason?

Hi Dario, Thanks for reporting!

The test isn't exactly written wrong, but it could be improved. I added two more assertions to catch the missing WDR. The new assertions make sure that the CPU resets at the right time, and not before.