Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Unit test tsc fixes part 15 #8104

Merged
merged 6 commits into from
Mar 22, 2022

Conversation

kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented Mar 21, 2022

Huge diff due to giant snapshot in MPollBody - a wrapper was removed from the snapshot, meaning every line has changed indent and confused github.


This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://pr8104--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

Kerry Archibald added 6 commits March 21, 2022 18:32
Signed-off-by: Kerry Archibald <kerrya@element.io>
Signed-off-by: Kerry Archibald <kerrya@element.io>
Signed-off-by: Kerry Archibald <kerrya@element.io>
Signed-off-by: Kerry Archibald <kerrya@element.io>
Signed-off-by: Kerry Archibald <kerrya@element.io>
Signed-off-by: Kerry Archibald <kerrya@element.io>
@kerryarchibald kerryarchibald requested a review from a team as a code owner March 21, 2022 18:08
@kerryarchibald kerryarchibald added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Mar 21, 2022
@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #8104 (a848018) into develop (0b80755) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #8104   +/-   ##
========================================
  Coverage    26.71%   26.71%           
========================================
  Files          872      872           
  Lines        52206    52206           
  Branches     13230    13230           
========================================
  Hits         13946    13946           
  Misses       38260    38260           

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally seems fine - thank you! If importing the components explodes violently, merge without changing it. I don't expect that they will explode, however.


jest.mock("../../../../src/settings/SettingsStore");

const _DateSeparator = sdk.getComponent("views.messages.DateSeparator");
const DateSeparator = TestUtils.wrapInMatrixClientContext(_DateSeparator);
const DateSeparator = sdk.getComponent("views.messages.DateSeparator");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to import this (makes de-skinning a lot easier)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay! i hate these


const CHECKED = "mx_MPollBody_option_checked";

const _MPollBody = sdk.getComponent("views.messages.MPollBody");
const MPollBody = TestUtils.wrapInMatrixClientContext(_MPollBody);
const MPollBody = sdk.getComponent("views.messages.MPollBody");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import please

@kerryarchibald
Copy link
Contributor Author

@turt2live unfortunately importing those components breaks the tests, so I'll leave that out of this change.
Attempted to get a component (${name}) before a skin has been loaded.
Will look into why/how to fix that in my next set of changes

@kerryarchibald kerryarchibald merged commit abc225d into develop Mar 22, 2022
@kerryarchibald kerryarchibald deleted the kerryarchibald/ts-test-rest-components branch March 22, 2022 10:32
@turt2live
Copy link
Member

It'll be because skinning is terrible, so would recommend waiting for that system to just leave (which will swing by and fix it anyhow)

turt2live added a commit that referenced this pull request Mar 22, 2022
This is a conflict between #8104 and #8084
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants