Skip to content

Conversation

@kerryarchibald
Copy link
Contributor

@kerryarchibald kerryarchibald commented Sep 29, 2022

The final instalment of test typescriptification 🥳

Fixes #2353

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@kerryarchibald kerryarchibald added the T-Task Tasks for the team like planning label Sep 29, 2022
@kerryarchibald kerryarchibald requested a review from a team as a code owner September 29, 2022 14:36
@kerryarchibald kerryarchibald marked this pull request as draft September 29, 2022 15:56
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.

clearing code review while draft

@kerryarchibald kerryarchibald marked this pull request as ready for review September 30, 2022 10:11
Copy link
Contributor

@duxovni duxovni left a comment

Choose a reason for hiding this comment

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

The expects that can be compressed into one line are just a nitpick, mostly focused on the un-cleaned-up httpBackends and the ==s.

// Alice should be tracking bob's device list
expect(bobStat).toBeGreaterThan(
0, "Alice should be tracking bob's device list",
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

May as well just make this a single line

// Alice should have marked bob's device list as untracked
expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked",
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise

// Alice should have marked bob's device list as untracked
expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked",
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

^

// Alice should have marked bob's device list as untracked
expect(bobStat).toEqual(
0, "Alice should have marked bob's device list as untracked",
0,
Copy link
Contributor

Choose a reason for hiding this comment

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

^

Comment on lines 103 to 105
expect(expectedEventTypes.indexOf(event.getType())).not.toEqual(
-1, "Recv unexpected event type: " + event.getType(),
-1,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can also be a one-liner now

Comment on lines 122 to 124
expect(expectedEventTypes.length).toEqual(
0, "Expected to see event types: " + expectedEventTypes,
0,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

And here

@kerryarchibald kerryarchibald requested a review from duxovni October 4, 2022 08:16
Copy link
Contributor

@duxovni duxovni left a comment

Choose a reason for hiding this comment

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

Looks good, there's still the == here but it's a minor nitpick in the context of a test.

Comment on lines +74 to 75
if (!(req.queryParams?.access_token == accessToken ||
req.headers["Authorization"] == "Bearer " + accessToken)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

^

@kerryarchibald kerryarchibald merged commit a1b046b into develop Oct 6, 2022
@kerryarchibald kerryarchibald deleted the kerry/ts-jest-integ branch October 6, 2022 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Task Tasks for the team like planning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert all matrix-js-sdk tests to use Typescript

4 participants