-
Notifications
You must be signed in to change notification settings - Fork 232
Move some test functionality into new backend-test-support package #8135
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
base: master
Are you sure you want to change the base?
Conversation
This allows them to use `@itwin/test-support`
|
This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 |
|
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.) |
…s-core into tcobbs/test-support
|
This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 |
|
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. |
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. |
|
I renamed the package to |
|
This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 |
|
/azp run iTwin.js |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This pull request is now in conflicts. Could you fix it @tcobbs-bentley? 🙏 |
The
HubMockclass 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-supportpackage that containsHubMockin preparation for it being made public (or more likely beta in the short term). Classes that depend onHubMockwere also put intobackend-test-support.Unfortunately, removing these items from
core/backend/src/testwould result in failed core-backend unit tests, since those tests rely on this functionality, andbackend-test-supportdepends on@itwin/core-backend. I verified that everything outsidecore-backendis now usingbackend-test-supportby temporarily deletingcore/backend/src/test.I have now created a new core/backend-tests directory that houses a new (unpublished)
@itwin/core-backend-testspackage. 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.