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

react to breaking change in 3.0 wrt ecsql query row format (backport #3257) #3259

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Feb 23, 2022

This is an automatic backport of pull request #3257 done by Mergify.


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

Co-authored-by: Arun George <[email protected]>
(cherry picked from commit d1db893)
@pmconne
Copy link
Member

pmconne commented Feb 23, 2022

Same comment as from original PR: How was this not caught by tests and should there be a test that would have caught it / will catch future regressions?

@aruniverse
Copy link
Member

Same comment as from original PR: How was this not caught by tests and should there be a test that would have caught it / will catch future regressions?

Sorry PR auto merged before I could reply. It wasn't caught due to all the mocking we due, and not testing against an actual iModel connection. Not sure how we'd catch this in the future outside of setting up some e2e to test against an actual iModel connection. @saskliutas any ideas?

@saskliutas
Copy link
Member

Same comment as from original PR: How was this not caught by tests and should there be a test that would have caught it / will catch future regressions?

Sorry PR auto merged before I could reply. It wasn't caught due to all the mocking we due, and not testing against an actual iModel connection. Not sure how we'd catch this in the future outside of setting up some e2e to test against an actual iModel connection. @saskliutas any ideas?

There are some tests that are testing ModelsTree using real iModel. I will try add some for CategoriesTree too.

@aruniverse
Copy link
Member

aruniverse commented Feb 23, 2022

I will try add some for CategoriesTree too.

I assume this will be in a separate pr? Because it also needs to get backported back to master

@aruniverse aruniverse enabled auto-merge (squash) February 23, 2022 20:26
@aruniverse aruniverse merged commit fa71d35 into release/3.0.x Feb 23, 2022
@aruniverse aruniverse deleted the mergify/bp/release/3.0.x/pr-3257 branch February 23, 2022 20:55
@saskliutas
Copy link
Member

I will try add some for CategoriesTree too.

I assume this will be in a separate pr? Because it also needs to get backported back to master

Yes I will open a new PR for test.

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.

5 participants