Try to fix the test problems after the async changes #3061
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## develop #3061 +/- ##
===========================================
+ Coverage 23.00% 23.10% +0.10%
===========================================
Files 52 52
Lines 11578 11582 +4
===========================================
+ Hits 2663 2676 +13
+ Misses 8915 8906 -9
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Using your PR I still get these messages: Also, the tests still fail sometimes locally for me :-( Any luck on your side maybe? |
|
Oh, I just looked at the e2e test and ignored the others. I'll take a look at the others. |
Is that an indication of the error? I get the same message in the master branch. At the beginning I also had locally an error message three times during the electron tests. After the last commit this didn't happen anymore. Since the error only occurs very rarely, I'm not sure if it really makes a difference. |
this is hopefully a fix for the electron tests :)
no, this is no indication of the error and this is printed after every test run (we could add --detectOpenHandles which would suppress this message) We have to use tests hanging, needing --forceExit (all these tests are using `waitForElement`):
- tests/e2e/modules_empty_spec.js
- tests/e2e/env_spec.js
- tests/e2e/modules/weather_current_spec.js
- tests/e2e/modules/weather_forecast_spec.js
- tests/e2e/modules/newsfeed_spec.js
- tests/e2e/modules/clock_spec.js
- tests/e2e/modules/calendar_spec.js
- tests/e2e/modules/compliments_spec.js
- tests/e2e/modules/alert_spec.js
- tests/e2e/modules/weather_hourly_spec.js
- tests/e2e/modules/clock_es_spec.js
tests ending correctly not using `waitForElement`:
+ tests/e2e/template_spec.js
+ tests/e2e/port_spec.js
+ tests/e2e/ipWhitelist_spec.js
+ tests/e2e/fonts_spec.js
+ tests/e2e/vendor_spec.js
+ tests/e2e/translations_spec.js
tests ending correctly using `waitForElement`:
+ tests/e2e/modules_position_spec.js
+ tests/e2e/modules_display_spec.js
+ tests/e2e/modules/helloworld_spec.jsThe e2e tests are still failing as before with this PR :( Because of these errors ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From tests/e2e/modules/calendar_spec.js.
22 | // return nodefetch(url, options);
23 | // }
> 24 | const nodefetch = await require("node-fetch");
| ^
25 | return nodefetch(url, options);
26 | }
27 |
at require (js/fetch.js:24:26)
at fetchCalendar (modules/default/calendar/calendarfetcher.js:63:17)
at Timeout.<anonymous> (modules/default/calendar/calendarfetcher.js:98:7)
ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From tests/e2e/modules/newsfeed_spec.js.
22 | // return nodefetch(url, options);
23 | // }
> 24 | const nodefetch = await require("node-fetch");
| ^
25 | return nodefetch(url, options);
26 | }
27 |
at require (js/fetch.js:24:26)
at fetchNews (modules/default/newsfeed/newsfeedfetcher.js:92:5)
at Timeout.<anonymous> (modules/default/newsfeed/newsfeedfetcher.js:114:7)I deleted the calendar and newsfeed tests and without them all errors are gone, also the weather tests are working now (so I think the weather tests were affected because they ran after the above tests). So I will now dig into the calendar and newsfeed tests ... |
According to the jest documentation, this would have performance disadvantages. But my tests with this option didn't seem that long to me. > 24 | const nodefetch = await require("node-fetch");
| ^Is this is a sign that we should finally get rid of node-fetch 2? *justjoking* 😅 I've just tried a few more things, but haven't really made any progress. By the way, I'm impressed with all the tests you've created. Respect! 👏 |
|
So what about this PR @khassel , will this be obsolete soon too? |
yes, but may we need the fix for the electron test ... |
KristjanESPERANTO
force-pushed
the
develop
branch
from
b19f0aa to
5a3f172
Compare
KristjanESPERANTO
force-pushed
the
develop
branch
from
5a3f172 to
d276a7d
Compare
To fix the test problems after the async changes (#3050), I tried to find out the cause with help of the Jest option
--detectOpenHandleson the e2e tests. I got some error messages. And since I haven't had much experience with Jest yet, I threw some async and await keywords around until the error messages were gone. Probably not all changes make sense, but maybe this approach will help us.