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

rework unit tests (mocking the fetch()) #2183

Merged
merged 6 commits into from
Mar 15, 2024

Conversation

ftoromanoff
Copy link
Contributor

@ftoromanoff ftoromanoff commented Sep 7, 2023

This PR propose a rewrite of the unit tests:

  • minor corrections in tests
  • mocking of the 'fetch()' function to have unit tests to run independent from network
    • to do that the files that should have been fetch have been added localy.
    • we use sinon to mock itowns.Fetcher
  • add a file test for testing Fetcher.js

Modifications:

  • add 'sinon' library to do the mocking
  • all tests
    • add all test files in local folder test/data/unitTest
    • change global url to local ones
  • add fetcher.js to test Fetcher.js
    • add library xml dom for xml fetcher

@ftoromanoff ftoromanoff force-pushed the testMaster branch 7 times, most recently from 4af85a4 to be3598c Compare September 12, 2023 12:12
@ftoromanoff ftoromanoff changed the title Test master rework test unitaire (mocking the fetch()) Sep 12, 2023
@ftoromanoff ftoromanoff changed the title rework test unitaire (mocking the fetch()) rework unit tests (mocking the fetch()) Sep 14, 2023
@ftoromanoff ftoromanoff force-pushed the testMaster branch 11 times, most recently from c0b79b8 to de49764 Compare December 18, 2023 16:35
@ftoromanoff ftoromanoff force-pushed the testMaster branch 2 times, most recently from 749d412 to 849eaa5 Compare December 20, 2023 13:59
@ftoromanoff ftoromanoff force-pushed the testMaster branch 5 times, most recently from a6bf551 to 7ef7825 Compare January 5, 2024 09:38
@ftoromanoff ftoromanoff force-pushed the testMaster branch 5 times, most recently from fc9f25d to de43a0f Compare March 6, 2024 14:02
@ftoromanoff ftoromanoff requested review from jailln and removed request for jailln March 6, 2024 14:32
@jailln jailln requested a review from AnthonyGlt March 7, 2024 13:42
src/Layer/C3DTilesLayer.js Show resolved Hide resolved
test/unit/3dtileslayerprocessbatchtable.js Show resolved Hide resolved
test/unit/dataSourceProvider.js Outdated Show resolved Hide resolved
test/unit/demutils.js Outdated Show resolved Hide resolved
test/unit/featuregeometrylayer.js Outdated Show resolved Hide resolved
test/unit/entwine.js Outdated Show resolved Hide resolved
test/unit/orientedimagelayer.js Outdated Show resolved Hide resolved
};
let stubFetcherJson;
before(function () {
stubFetcherJson = sinon.stub(Fetcher, 'json')
Copy link
Contributor

@jailln jailln Mar 7, 2024

Choose a reason for hiding this comment

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

I think we could factorize a bit this mocking logic in one place (e.g. in utils.js) by creating the following method:

function getFetcherStub(method, returnVal) {
    sinon.stub(Fetcher, method)
    .callsFake(() => Promise.resolve(returnVal));
}

And just do stubFetcherJson = utils.getFetcherStub('json', tileset) here (and use this method everywhere else where you create stubs). WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of this PR is to get in the direction of real unti tests. Thus i did mock the fetching everywhere, but later on we should be mocking the process that use the mocked fetcher (except for some specifique tests).That's why I think that a it will be on some further PR to think about how to factorize the different mocking parts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, in this case could you open an issue about it please ?

@ftoromanoff ftoromanoff force-pushed the testMaster branch 6 times, most recently from 22dcc38 to f5e81ad Compare March 13, 2024 12:27
test/unit/orientedimagelayer.js Show resolved Hide resolved
test/unit/fetcher.js Outdated Show resolved Hide resolved
@Desplandis Desplandis merged commit 2fb3298 into iTowns:master Mar 15, 2024
9 checks passed
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.

4 participants