itwinjs-core icon indicating copy to clipboard operation
itwinjs-core copied to clipboard

Move some test functionality into new backend-test-support package

Open tcobbs-bentley opened this issue 9 months ago • 11 comments

The HubMock class is used outside itwinjs-core, even though it is marked as internal and not exported (requiring a deep import to access it). This PR creates a new @itwin/backend-test-support package that contains HubMock in preparation for it being made public (or more likely beta in the short term). Classes that depend on HubMock were also put into backend-test-support.

Unfortunately, removing these items from core/backend/src/test would result in failed core-backend unit tests, since those tests rely on this functionality, and backend-test-support depends on @itwin/core-backend. I verified that everything outside core-backend is now using backend-test-support by temporarily deleting core/backend/src/test.

I have now created a new core/backend-tests directory that houses a new (unpublished) @itwin/core-backend-tests package. I moved everything from core/backend/test into there, removing the files that are now in test-support. All tests pass, both locally and on CI. Additionally the iOS simulator tests run in the pipeline were updated to use core/backend-tests instead of core/backend.

tcobbs-bentley avatar May 22 '25 20:05 tcobbs-bentley

This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

mergify[bot] avatar Jun 06 '25 17:06 mergify[bot]

Why does this require a new package versus simply exporting relevant APIs from core-backend?

pmconne avatar Jun 09 '25 13:06 pmconne

Why does this require a new package versus simply exporting relevant APIs from core-backend?

I did this based on talks I had with Keith a few months ago, but I don't remember all of the exact details, so it could be that doing it this way is the wrong way to go.

The thought was that test-only functionality should be kept separate from core-backend, and in the future we will add things with (for example) mocha dependencies to the new package. Those would then not introduce mocha dependencies in core-backend. But looking at what is in there now, and looking at the package.json, I see that all it has are dev dependencies to chai, which already existed in core-backend. (As an aside, if this PR stands, then core-backend's dev dependencies to chai can be removed.)

tcobbs-bentley avatar Jun 09 '25 15:06 tcobbs-bentley

This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

mergify[bot] avatar Jun 10 '25 19:06 mergify[bot]

I'll defer to others on the virtues of separate test packages. But if you do go that route, please rename the tools/test-support package to indicate it's only usable for backend tests - everything in it except for AdvancedEquals depends on core-backend.

pmconne avatar Jun 11 '25 13:06 pmconne

But if you do go that route, please rename the tools/test-support package to indicate it's only usable for backend tests - everything in it except for AdvancedEquals depends on core-backend.

The hope was to move other non-backend test support functionality in there as well. That obviously hasn't been done yet, and I guess it's possible that it won't work. I can rename it to backend-test-support.

tcobbs-bentley avatar Jun 11 '25 14:06 tcobbs-bentley

I renamed the package to @itwin/backend-test-support. I talked to @wgoehrig, and he agreed that while this new package isn't an ideal solution, it's the best one available.

tcobbs-bentley avatar Jun 11 '25 15:06 tcobbs-bentley

This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

mergify[bot] avatar Jun 11 '25 17:06 mergify[bot]

/azp run iTwin.js

tcobbs-bentley avatar Jun 11 '25 18:06 tcobbs-bentley

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Jun 11 '25 18:06 azure-pipelines[bot]

This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

mergify[bot] avatar Jun 13 '25 01:06 mergify[bot]