espruino / BangleApps

Bangle.js App Loader (and Apps)
https://banglejs.com/apps
MIT License
498 stars 1.17k forks source link

Port additional tests and fix GB events in runapptests.js #3413

Closed halemmerich closed 6 months ago

halemmerich commented 6 months ago
gfwilliams commented 6 months ago

Ok, thanks - the 12 sec delay isn't great if we're trying to run this in a CI test...

I checked and the emulator just uses Date.now() to get the current time in milliseconds... I wonder if the test harness could just override that and allow an additional value to be added to what it returns?

halemmerich commented 6 months ago

Hm, overriding Date.now() on its own does not seem to influence timers.

setTimeout(()=>{print("waited");}, 20000);
setInterval(()=>{print("waited");}, 20000);

const delorean = (jump)=>{
  Date.now = (o)=>{return ()=>{
    return o() + jump;
  };}(Date.now);
  for(let c of global["\xff"].timers){
    if(c) c.time -= jump * 1000;
  }
};

print("Before", Date.now());
print("Before", global["\xff"].timers);

delorean(22000);

print("After", Date.now());
print("After", global["\xff"].timers);

This does seem to work and should be enough for my usecase but it feels massively hacky. No idea what other stuff gets broken by this. Might be enough for tests anyway.

gfwilliams commented 6 months ago

Ahh, I mean override Date.now in Node.js, not in Espruino!

But yes, messing with the timers like that is probably good enough for a test at the moment? Maybe it can be another command type and if it causes issues later we can extend it

halemmerich commented 6 months ago

I think just renaming sleep would be ok. Having sleep that actually pauses execution will eventually be used so maybe it would be better to not even allow that as long as there are ways around having to do this? I implemented it specifically to deal with waiting for timers. Maybe we just name it "speedup" or something like that?

I suspect overriding Date.now() in Node might have even bigger repercussions than doing it in the emulator. Edit: Tried it and somehow the Date.now() in the Emulator suddenly returns a negative value after overriding it in Node.

gfwilliams commented 6 months ago

Ok, thanks - this looks good!

I think sleep is fine for naming.

Looking at the code, you're already overwriting Date.now inside Espruino - could that be why you were getting negative values? That the time was being changed twice - both inside and out?

It's probably best not to overwrite Date.now in Espruino either way - it's not used internally, so if you do new Date().ms it wouldn't match Date.now(), and getTime() wouldn't match either. Having one change but not the others is probably more confusing than not doing it at all.

halemmerich commented 6 months ago

could that be why you were getting negative values?

I had removed the override inside before but it was just a quick and dirty test. There might've been something else wrong.

I have just removed the override for Date.now as my current tests don't need it. It is now named advanceTimers to keep sleep free for a more complete implementation when it is actually needed.

gfwilliams commented 6 months ago

Thanks! Looks great!