Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update weather tests #3008

Merged
merged 12 commits into from
Jan 26, 2023
Merged

Update weather tests #3008

merged 12 commits into from
Jan 26, 2023

Conversation

rejas
Copy link
Collaborator

@rejas rejas commented Jan 15, 2023

Added a few tests for sunset/sunrise and feelsLike
Lets see if they run through first...

@rejas rejas changed the base branch from master to develop January 15, 2023 22:47
@rejas rejas marked this pull request as ready for review January 15, 2023 22:47
@rejas
Copy link
Collaborator Author

rejas commented Jan 15, 2023

Tests failing, therefor back to the draft...

@rejas rejas marked this pull request as draft January 15, 2023 22:55
@rejas
Copy link
Collaborator Author

rejas commented Jan 24, 2023

Somhow the evaluate fucntion for mocking the Date here: https://github.com/MichMich/MagicMirror/blob/2fec314ff51667be54da59ccaf38991ad805d3f6/tests/electron/helpers/global-setup.js#L22
is never called @khassel therefore the time is never faked... Any idea why?

@khassel
Copy link
Collaborator

khassel commented Jan 24, 2023

is never called @khassel therefore the time is never faked... Any idea why?

no, I think it is called, otherwise weather and compliment tests would fail.

You have this error in the electron tests:

    TypeError: Cannot read properties of null (reading 'locator')

      40 | exports.getElement = async (selector) => {
      41 | 	expect(global.page);
    > 42 | 	let elem = global.page.locator(selector);

After digging around I found that the line jest.retryTimes(3); causes running the tests several times.
So please remove this line (can you do this in this PR? Otherwise I have to make another one ...) and run the tests again.

Now you are getting

 FAIL   electron  tests/electron/modules/weather_spec.js
  Weather module
    Current weather with sunrise
      ✕ should render sunrise (369 ms)
    Current weather with sunset
      ✓ should render sunset (234 ms)

  ● Weather module › Current weather with sunrise › should render sunrise

    expect(received).toBe(expected) // Object.is equality

    Expected: "7:00 am"
    Received: "3:45 pm"

which is may caused by your changes in this PR (tests are o.k. on develop).

@rejas rejas marked this pull request as ready for review January 26, 2023 07:24
@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2023

Codecov Report

Merging #3008 (a07b540) into develop (7198ae5) will increase coverage by 0.47%.
The diff coverage is 75.00%.

📣 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    #3008      +/-   ##
===========================================
+ Coverage    21.78%   22.25%   +0.47%     
===========================================
  Files           52       52              
  Lines        11459    11499      +40     
===========================================
+ Hits          2496     2559      +63     
+ Misses        8963     8940      -23     
Impacted Files Coverage Δ
modules/default/weather/weatherutils.js 80.99% <52.17%> (+3.44%) ⬆️
modules/default/updatenotification/git_helper.js 89.58% <83.33%> (-0.95%) ⬇️
modules/default/weather/weatherobject.js 69.50% <94.11%> (+15.08%) ⬆️
modules/default/updatenotification/node_helper.js 83.33% <0.00%> (+5.55%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rejas
Copy link
Collaborator Author

rejas commented Jan 26, 2023

Found the "issue" with tests. Introduced it myself in the first commit :-) Had to work around to get electron and playwright tests both work out. But now its ready for review :-)

@khassel khassel merged commit a8dc563 into MagicMirrorOrg:develop Jan 26, 2023
@rejas rejas deleted the precipation branch January 26, 2023 18:49
MichMich added a commit that referenced this pull request Apr 4, 2023
## [2.23.0] - 2023-04-04

Thanks to: @angeldeejay, @buxxi, @CarJem, @dariom, @DaveChild, @dWoolridge, @grenagit, @Hirschberger, @KristjanESPERANTO, @MagMar94, @naveensrinivasan, @nfogal, @psieg, @rajniszp, @retroflex, @SkySails and @tomzt.

Special thanks to @khassel, @rejas and @sdetweil for taking over most (if not all) of the work on this release as project collaborators. This version would not be there without their effort. Thank you guys! You are awesome!

### Added

- Added increments for hourly forecasts in weather module (#2996)
- Added tests for hourly weather forecast
- Added possibility to ignore MagicMirror repo in updatenotification module
- Added Pirate Weather as new weather provider (#3005)
- Added possibility to use your own templates in Alert module
- Added error message if `<modulename>.js` file is missing in module folder to get a hint in the logs (#2403)
- Added possibility to use environment variables in `config.js` (#1756)
- Added option `pastDaysCount` to default calendar module to control of how many days past events should be displayed
- Added thai language to alert module
- Added option `sendNotifications` in clock module (#3056)

### Removed

- Removed darksky weather provider
- Removed unneeded (and unwanted) '.' after the year in calendar repeatingCountTitle (#2896)

### Updated

- Use develop as target branch for dependabot
- Update issue template, contributing doc and sample config
- The weather modules clearly separates precipitation amount and probability (risk of rain/snow)
  - This requires all providers that only supports probability to change the config from `showPrecipitationAmount` to `showPrecipitationProbability`.
- Update tests for weather and calendar module
- Changed updatenotification module for MagicMirror repo only: Send only notifications for `master` if there is a tag on a newer commit
- Update dates in Calendar widgets every minute
- Cleanup jest coverage for patches
- Update `stylelint` dependencies, switch to `stylelint-config-standard` and handle `stylelint` issues, update `main.css` matching new rules
- Update Eslint config, add new rule and handle issue
- Convert lots of callbacks to async/await
- Revise require imports (#3071 and #3072)

### Fixed

- Fix wrong day labels in envcanada forecast (#2987)
- Fix for missing default class name prefix for customEvents in calendar
- Fix electron flashing white screen on startup (#1919)
- Fix weathergov provider hourly forecast (#3008)
- Fix message display with HTML code into alert module (#2828)
- Fix typo in french translation
- Yr wind direction is no longer inverted
- Fix async node_helper stopping electron start (#2487)
- The wind direction arrow now points in the direction the wind is flowing, not into the wind (#3019)
- Fix precipitation css styles and rounding value
- Fix wrong vertical alignment of calendar title column when wrapEvents is true (#3053)
- Fix empty news feed stopping the reload forever
- Fix e2e tests (failed after async changes) by running calendar and newsfeed tests last
- Lint: Use template literals instead of string concatenation
- Fix default alert module to render HTML for title and message
- Fix Open-Meteo wind speed units
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants