search

iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
600 stars 210 forks source link

open default view in test-apps #7037

Open aruniverse opened 1 month ago

aruniverse commented 1 month ago 
          I know you added this test to catch this next time, but wonder if we should update dta/dpta to open up default view.

That can be a separate pr, just mentioning it here so this doesnt get lost.

_Originally posted by @aruniverse in https://github.com/iTwin/itwinjs-core/pull/7035#discussion_r1702025747_

pmconne commented 1 month ago

A keyin to switch to a default-created view and/or a config option to use a default-created view at startup both sound fine for display-test-app. The folks who use display-test-app on a daily basis like the way it currently works though - I'd prefer if it continues to work the current way by default.

What's the use case for display-performance-test-app?

aruniverse commented 1 month ago

What's the use case for display-performance-test-app?

I guess the case here isnt as strong if we keep the default behavior of opening up to a specific view. The image tests were run a few times since the pr merged so we kinda expected this to have been caught through that. (assuming the app works like most of our prod apps which by default open up to the default view)

Is there a reason why default views are used in those image tests?

pmconne commented 1 month ago

Is there a reason why default views are used in those image tests?

(I'm assuming you meant "aren't"). Most of the ImageTests views were created to test very specific aspects of the display system. It has an option that says "test all the views of this iModel". We could make that option also test a default-created view.

TBH though, this regression should have been caught by unit tests for ViewCreator3d.