fix: fix tests using the new sdk #1665

martin-trajanovski · 2024-11-19T15:25:24Z

Description

Fixing e2e and unit tests with the new sdk

Motivation

E2e and unit tests failing after the new sdk migration.

Tests included

Included for each change/fix?
Passing? (Merge will not be approved unless this is checked)

Documentation

swagger documentation updated [required]
official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

Does it require a specific version of the backend
which version of the backend is required:

Summary by Sourcery

Bug Fixes:

Fix tests to use the new SDK by updating service imports and method calls across multiple test files.

Summary by Sourcery

Fix tests to work with the new SDK by updating service imports and method calls. Refactor Cypress commands for better token handling. Update test files to ensure compatibility with the new SDK structure.

Bug Fixes:

Fix tests to use the new SDK by updating service imports and method calls across multiple test files.

Enhancements:

Refactor Cypress commands to use a new method for retrieving tokens, improving code readability and maintainability.

Tests:

Update test files to align with the new SDK structure, ensuring compatibility and functionality.

…ontend into new-sdk-tests-fix

… for now

sourcery-ai

Hey @martin-trajanovski - I've reviewed your changes - here's some feedback:

Overall Comments:

There are some TODO comments about fixing types that should be addressed in follow-up PRs, but they don't block the core functionality

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟡 Complexity: 2 issues found
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

src/app/shared/MockStubs.ts

src/app/datasets/dataset-detail/dataset-detail.component.spec.ts

…i bot suggestion

Bumps the types group with 1 update: [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node). Updates `@types/node` from 22.9.0 to 22.9.3 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-type: direct:development update-type: version-update:semver-patch dependency-group: types ... Signed-off-by: dependabot[bot] <support@github.com>

…ypes-c0e67af0c7 chore(deps-dev): bump @types/node from 22.9.0 to 22.9.3 in the types group

Bumps [cypress](https://github.com/cypress-io/cypress) from 13.15.2 to 13.16.0. - [Release notes](https://github.com/cypress-io/cypress/releases) - [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md) - [Commits](cypress-io/cypress@v13.15.2...v13.16.0) --- updated-dependencies: - dependency-name: cypress dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

…ypress-13.16.0 chore(deps-dev): bump cypress from 13.15.2 to 13.16.0

Bumps [mathjs](https://github.com/josdejong/mathjs) from 13.2.2 to 14.0.0. - [Changelog](https://github.com/josdejong/mathjs/blob/develop/HISTORY.md) - [Commits](josdejong/mathjs@v13.2.2...v14.0.0) --- updated-dependencies: - dependency-name: mathjs dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>

…athjs-14.0.0 chore(deps): bump mathjs from 13.2.2 to 14.0.0

Junjiequan

Looks good in general.
I stopped tracking the use case of createMock where the mockData has already been defined in the MockStubs.ts file. It would be easier to track them with search in vscode

src/app/datasets/dataset-detail/dataset-detail.component.spec.ts

Junjiequan · 2024-11-27T12:55:28Z

src/app/datasets/dataset-table/dataset-table.component.spec.ts

@@ -131,7 +136,7 @@ describe("DatasetTableComponent", () => {

  describe("#userErrorCondition()", () => {
    it("should return true if dataset has missingFilesError", () => {
-      const dataset = new Dataset();
+      const dataset = createMock<DatasetClass>({});


Wouldn't it better if we use DatasetClass as the default for mockDataset and use OutputDatasetObsoleteDto for edge cases? What do you think?

Junjiequan · 2024-11-27T14:06:30Z

src/app/datasets/share-dialog/share-dialog.component.spec.ts

+      // TODO: Fix this any type casting here
+      spyOn(
+        component.userIdentititiesService,
+        "userIdentitiesControllerIsValidEmail",
+      ).and.returnValue(of(true) as any);


Is type any needed here?

src/app/proposals/view-proposal-page/view-proposal-page.component.spec.ts

src/app/publisheddata/publisheddata-dashboard/publisheddata-dashboard.component.spec.ts

Junjiequan · 2024-11-27T14:16:37Z

src/app/shared/MockStubs.ts

+  return data as T;
+}
+
+export const mockDataset = createMock<OutputDatasetObsoleteDto>({});


As mentioned above, wouldn't it make more sense to use DatasetClass instead of ObsoleteDto?

As I mentioned below I am not focusing that much on the details here in the tests right now as we will refactor all this in the near future if we use the next version of the dataset endpoints. They will be typed correctly and we wont have these inconsistencies anymore

Junjiequan · 2024-11-27T14:28:07Z

src/app/shared/services/thumbnail.service.spec.ts

+    // TODO: Try to fix any type casting here
+    datasetApi.datasetsControllerThumbnail.and.returnValue(
+      of({ thumbnail } as any),
+    );


maybe something like of({ thumbnail } as unknown as HttpResponse<Attachment>)?

I left a TODO comment here and don't want to investigate the proper fix right now as it is in the tests and the tests are passing. Will try to do it later and fix most of the TODOs left with this PR in the future whenever we can get some extra time doing this. I think I spent a lot of time around the types and most of them at least in the code are fixed. In the tests we can check it later I hope. But thanks for the feedback @Junjiequan I appreciate it.

Junjiequan · 2024-11-27T14:29:33Z

src/app/shared/services/thumbnail.service.spec.ts

  });

  it("should fetch thumbnail and cache it if not already cached", async () => {
    const pid = "test-pid";
    const thumbnail = "http://thumbnail-url.com/image.jpg";

-    datasetApi.thumbnail.and.returnValue(of({ thumbnail }));
+    datasetApi.datasetsControllerThumbnail.and.returnValue(
+      of({ thumbnail } as any),


same as above

src/app/state-management/actions/datasets.actions.spec.ts

src/app/state-management/actions/proposals.actions.spec.ts

auto size login logo div

martin-trajanovski added 16 commits November 19, 2024 16:21

start fixing tests with the new sdk

f54b1db

add needed stub classes and fix some more tests

1efa821

Merge branch 'new-sdk-release' of https://github.com/SciCatProject/fr…

a98ff14

…ontend into new-sdk-tests-fix

continue fixing unit tests

464ba8c

try to fix e2e tests and revert some changes that need more attention…

3c54858

… for now

changes to just run the tests

066f6f7

use latest sdk

db0eecf

update package-lock file

4f37b80

fixing unit tests

76bd6fd

fix more unit tests

37260d3

continue fixing tests

f63b432

update the sdk

fb4fc01

fix last e2e test

8d38fda

fix thumbnail unit tests

24dcebb

revert some change

047afbf

finalize fixing unit tests

dcff2ab

martin-trajanovski marked this pull request as ready for review November 25, 2024 14:33

martin-trajanovski requested review from Junjiequan and nitrosx November 25, 2024 14:33

sourcery-ai bot reviewed Nov 25, 2024

View reviewed changes

src/app/shared/MockStubs.ts Outdated Show resolved Hide resolved

src/app/datasets/dataset-detail/dataset-detail.component.spec.ts Outdated Show resolved Hide resolved

martin-trajanovski and others added 8 commits November 25, 2024 15:34

revert the backend image changes after the tests pass

76c8fcf

add some improvements in the mocked objects for unit tests based on a…

59a0ed7

…i bot suggestion

Merge pull request #1668 from SciCatProject/dependabot/npm_and_yarn/t…

2f34ab0

…ypes-c0e67af0c7 chore(deps-dev): bump @types/node from 22.9.0 to 22.9.3 in the types group

Merge pull request #1670 from SciCatProject/dependabot/npm_and_yarn/c…

f94ca4c

…ypress-13.16.0 chore(deps-dev): bump cypress from 13.15.2 to 13.16.0

Merge pull request #1671 from SciCatProject/dependabot/npm_and_yarn/m…

06b4b7e

…athjs-14.0.0 chore(deps): bump mathjs from 13.2.2 to 14.0.0

martin-trajanovski self-assigned this Nov 26, 2024

martin-trajanovski added the testing label Nov 26, 2024

martin-trajanovski added the DCS DAPHNE Contribution to SciCat label Nov 26, 2024

martin-trajanovski added 2 commits November 27, 2024 13:25

remove encodeURIComponent in the effects as it seems redundant

8bf15e2

fix test files after some changes

4cc40a8

Junjiequan reviewed Nov 27, 2024

View reviewed changes

cfelder and others added 3 commits November 27, 2024 15:54

fix: center logo in LoginDialog (#1656)

5f490c0

auto size login logo div

try to use mock objects as much as possible

e5f9cd8

resolve merge conflicts

51a5e5e

nitrosx approved these changes Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fix tests using the new sdk #1665

fix: fix tests using the new sdk #1665

martin-trajanovski commented Nov 19, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot left a comment

Junjiequan left a comment

Junjiequan Nov 27, 2024

Junjiequan Nov 27, 2024

Junjiequan Nov 27, 2024

martin-trajanovski Nov 28, 2024

Junjiequan Nov 27, 2024

martin-trajanovski Nov 28, 2024

Junjiequan Nov 27, 2024 •

edited

Loading

fix: fix tests using the new sdk #1665

Are you sure you want to change the base?

fix: fix tests using the new sdk #1665

Conversation

martin-trajanovski commented Nov 19, 2024 • edited by sourcery-ai bot Loading

Description

Motivation

Tests included

Documentation

official documentation info

Backend version

Summary by Sourcery

Summary by Sourcery

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Junjiequan left a comment

Choose a reason for hiding this comment

Junjiequan Nov 27, 2024

Choose a reason for hiding this comment

Junjiequan Nov 27, 2024

Choose a reason for hiding this comment

Junjiequan Nov 27, 2024

Choose a reason for hiding this comment

martin-trajanovski Nov 28, 2024

Choose a reason for hiding this comment

Junjiequan Nov 27, 2024

Choose a reason for hiding this comment

martin-trajanovski Nov 28, 2024

Choose a reason for hiding this comment

Junjiequan Nov 27, 2024 • edited Loading

Choose a reason for hiding this comment

martin-trajanovski commented Nov 19, 2024 •

edited by sourcery-ai bot

Loading

Junjiequan Nov 27, 2024 •

edited

Loading