Update the Drupal\KernelTests\Core\Database\LoggingTest a little to make it pass for SQL Server

Created on 28 October 2021, about 3 years ago
Updated 20 February 2023, over 1 year ago

Problem/Motivation

The Drupal\KernelTests\Core\Database\LoggingTest now fails for SQL Server, because SQL Server runs an extra query when the query is not to the default schema. The extra query makes to total number of run queries 1 higher. In the LoggingTest a number of times there are assertions to the number of ran queries.

Proposed resolution

Change the assertions in a couple of places from assertCount() to assertGreaterThanOrEqual().

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

πŸ“Œ Task
Status

Needs work

Version

10.1 ✨

Component
DatabaseΒ  β†’

Last updated 2 days ago

  • Maintained by
  • πŸ‡³πŸ‡±Netherlands @daffie
Created by

πŸ‡³πŸ‡±Netherlands daffie

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Triggering for D10

  • Status changed to RTBC almost 2 years ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Passed D10 and looks like a good simple change.

  • Status changed to Needs work almost 2 years ago
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Can you add a comment as to why this could be greater than, so we don't accidentally undo it in the future? Also should we assert that the number of queries is either N or N+1, rather than allowing any larger number?

  • Status changed to Needs review almost 2 years ago
  • πŸ‡³πŸ‡±Netherlands daffie

    Added the requested comments.

  • Status changed to RTBC almost 2 years ago
  • πŸ‡³πŸ‡±Netherlands arantxio Dordrecht

    The requested comments have been added.

    Also should we assert that the number of queries is either N or N+1, rather than allowing any larger number?

    We currently do not have a clean assertion for a number range so testing for this would mean we have to add logic for this which could make the code look less readable.

    So unless we have something for that for me it's back to RTBC.

  • Status changed to Needs review over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Do we need to move this to a base class, and then allow individual drivers to override it? Then core database drivers could keep the stricter assertion.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    @catch could that be done here or in a follow up?

  • Status changed to Needs work over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    to address #12. Can this added to the base class?

Production build 0.71.5 2024