Change default compat level to 150 #34319
Conversation
There was a problem hiding this comment.
OK, I do have an idea for how to make the tests better...
In tests where the SQL varies based on version, we could check what actual version we're running against (on TestEnvironment), and do a conditional AssertSql with the two SQLs; aside from removing the duplication, this would also show very nicely exactly where we have variations across versions...
Ideally, in CI we'd even run the tests twice - once against 2019 and once against 2022. But even if we don't (for now), we'll have 2019 in CI, and 2022 in local runs.
What do you think? Am improving as-is to not block on this, but I think it should be a pretty trivial think to do if you're up for it...
| select (from l3 in ss.Set<Level3>() | ||
| select l3).Distinct().Skip(1).OrderBy(e => e.Id).FirstOrDefault().Name); | ||
|
|
||
| AssertSql( |
There was a problem hiding this comment.
If we do keep the test suite duplication, one idea to make this lighter is to not do SQL assertions except for those tests where there's a difference
|
I think we should discuss the testing some more. There are test infrastructure aspects to this. For example, we can't easily switch back and forth in a normal fixture, because the compat level is a singleton option. Also, I think there is value in asserting the SQL in both places, to catch regressions where we switch to using a 2022 feature in the 2019 cases, for example. The main thing is that we need better support for this in the test infrastructure. And we should consider both which variations add the most value to run as well as reducing the duplication in test overrides and baselines. |
I agree, I'm just saying that we can get full coverage for both SQL variants by running builds in CI (against different SQL Server versions), rather than attempting to have both variants handled inside the same test run, via different test classes. This also would provide better cover in that we'd actually test against 2019 and 2022, as opposed to running the two SQL variants against 2022. In he PG provide, for example, we run a CI matrix against all supported versions of PG, with slight variations in SQL generation which are handled this way. Of course, doing this fully assumes we run both 2019 and 2022 configs in CI, which we still don't have (but I think we should go in this direction).
I don't think that's needed in what I'm proposing: the (only) test suite would check which database it's running against (by looking at TestEnvironment, which checks the actual database version), and then configure only that version in UseSqlServer(). This way each CI config (of which there would be two) would only ever execute a single SQL version against its intended actual database version. Having said all that, I think it's perfectly fine to merge this PR as-is and clean up later! |
Fixes #34316