iTwin
Create Mesh Export Service Integration Test
Create an integration test which calls the MES using a test user and determines if the tile version of the requested model is supported by the frontend under which the test is running. The test will:
- Start an export from the MES
- Once that export is complete, obtain the tileset from that export
- Check the tile version of the provided tileset
- Check if the frontend under which the test is running will support rendering the tileset
While this test in and of itself is not the most useful or robust test, it serves two major purposes. Once the integration testing framework is set up, it will be significantly easier to expand upon the test that is here and create more robust tests. Additionally, the test can serve as an example to other teams of a way tile version compatibility can be checked in their own use cases.
Where will this test be run? As part of the iTwin.js Integration pipeline, or an external repo?
Where will this test be run? As part of the iTwin.js Integration pipeline, or an external repo?
Currently undecided. Most likely not iTwin.js integration pipeline.
Where will this test be run? As part of the iTwin.js Integration pipeline, or an external repo?
Currently undecided. Most likely not iTwin.js integration pipeline.
Does it really make sense to have this in this repo then?
Does it really make sense to have this in this repo then?
That's part of the discussion we're still having. Based on conversations with @arvindsvenkat, we will most likely want it to be an automated test, but we aren't set on where exactly, yet. Open to any suggestions.
That's part of the discussion we're still having. Based on conversations with @arvindsvenkat, we will most likely want it to be an automated test, but we aren't set on where exactly, yet. Open to any suggestions.
Based on the imports used within the integration test, you should be able to set up this integration test in a different repo. There's no internal APIs used that prevents you from doing this outside of the core mono repo. To setup a pipeline that runs the integration tests, you can use the template we have here as a reference.
Where will this test be run? As part of the iTwin.js Integration pipeline, or an external repo?
Currently undecided. Most likely not iTwin.js integration pipeline.
Does it really make sense to have this in this repo then?
@aruniverse, This test is intended to prevent the error that was encountered when the export tile version changed and wasn't supported by the iTwin viewer. I would assume it's directly correlated to upgrades in the iTwin.js version and the Mesh Export Service exports.
In my opinion, it should be run in the iTwin.js Integration test pipeline and also in the Mesh Export Service release pipeline (possibly scheduled in QA).
cc: @andremig-bentley @markschlosseratbentley , @danieliborra
When exactly will this test actually run?
rush test:integration expects a script named test:integration, which your package.json lacks.
The iTwin.js Integration - GitHub job runs a hard-coded set of integration tests, which doesn't include this one.
When exactly will this test actually run?
rush test:integrationexpects a script namedtest:integration, which your package.json lacks. The iTwin.js Integration - GitHub job runs a hard-coded set of integration tests, which doesn't include this one.
After further discussions, we have decided to add this test to the iTwin.js integration pipeline. The command to run the test has been changed to test:integration:tileversion to follow the general integration test naming conventions in core, but we don't want it to run automatically with rush test:integration, so that command does not exist in this package. As of now, this PR will only include the addition of the test, and the pipeline integration will be handled in a later PR.
Additionally, based on discussions with @arvindsvenkat and @markschlosseratbentley, the test has been changed to begin by kicking off an export, then polling for the status of that export, and once it is complete, going forward with the tile version testing on that new export, ensuring we get the latest tile version information from the MES.
where is this actually going to run? i dont see any pipeline changes. you can grep for this within the repo and look at the other tests as an example
I had the same question. @andremig-bentley later stated above:
After further discussions, we have decided to add this test to the iTwin.js integration pipeline. [...snip...] The command to run the test has been changed to test:integration:tileversion to follow the general integration test naming conventions in core, but we don't want it to run automatically with rush test:integration, so that command does not exist in this package. As of now, this PR will only include the addition of the test, and the pipeline integration will be handled in a later PR.
I'm not sure where the further discussions occurred or whether you were involved in them. I was not. It would be useful to understand why they decided to include the test in itwinjs-core after all, why they don't want to run it as part of rush test:integration, etc.
where is this actually going to run? i dont see any pipeline changes. you can grep for this within the repo and look at the other tests as an example
We had decided to have this PR include only the addition of the test, with pipeline changes coming later as time allowed. If you think we should roll it all into this PR, that can be done.
@pmconne @aruniverse To clear up any confusion, since the goal of this test is to validate the interoperability between the frontend here and the MES backend, we have decided that this test would be useful to run in two locations: As part of the Mesh Export Service Release Pipeline, and pertinent to this PR, in itwinjs-core. We think it is useful to run it here to validate that any frontend changes do not unintentionally break this connection.
Initially, we had chosen to only include it in the iTwinjs Integration - Github job as a matter of simplicity. However after discussing more with @markschlosseratbentley, we have decided that we will add it as well to rush test:integration as it would be just as useful there.
Additionally, we had also considered having this PR only include the actual addition of the test and not the changes to the pipeline etc. needed to add the test to the iTwinjs Integration - Github job and rush test:integration due to prioritization of other work. Our thought being we could finish this PR, have the test in place, and work on actually connecting it to the correct testing infrastructure as time allowed. However, it seems that it would make more sense to have this PR include all necessary changes for the test to run, so those changes are now WIP and will be included in this PR.
Due to this change in plan and the PR now being considered WIP, I am converting it back to draft, so feel free to unsubscribe for now, I'll re-request reviews when it's ready. Let me know if there are any other questions I can answer in the mean time.
This PR now includes the necessary changes to have the integration test run with rush test:integration and as a part of the iTwin.js Integration - Github job. It is now being marked again as ready for review.
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request is now in conflicts. Could you fix it @andremig-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](https://avatars.githubusercontent.com/in/10562?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request is now in conflicts. Could you fix it @andremig-bentley? 🙏 To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/
This PR will be closed with this comment. This is due to significant scope-creep, a very large amount of conversation as we narrowed in on what we wanted this PR to accomplish that will be difficult to decipher by someone in the future, and the fact that it has existed for an incredibly long time untouched, has gotten stale, and accumulated some significant conflicts. A new PR will be created soon that will be much cleaner from the start and will better serve us in the future.