botpress / messaging

Botpress messaging server
42 stars 37 forks source link

fix(messaging): fix inject error #551

Closed allardy closed 1 year ago

allardy commented 1 year ago

Pretty sure the error is related to the "isWebchatEvent" function... It was really too basic. image

These event causes those errors: image

Still need a way to test them correctly, since stratus uses the CDN, I didn't test "strictly", but logic looks fine.

allardy commented 1 year ago

https://linear.app/botpress/issue/STU-44/fix-annoying-error-coming-injectts

spg commented 1 year ago

Adding tests for that function would be pretty easy. Should we do this now?

allardy commented 1 year ago

Agreed that tests would be nice, but i'm not really used to DOM testing. Could be a good challenge for someone who implements new features in the future

spg commented 1 year ago

I was not thinking of DOM testing, but only testing some functions of inject.ts file. Here, the isWebchatEvent() could benefit from a few simple tests

allardy commented 1 year ago

@spg Tests added.

I disabled the firefox test because it's broken and has been for a long time, 6h running time is abusive. Fixing the real error should be done in a sepatate pr

image image

spg commented 1 year ago

@allardy please add a ticket in Linear so we remember to fix the tests