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

tests: Use direct values instead of raw SQL #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamahuman
Copy link
Contributor

Supersedes #11.

In order to prevent (-1) from being masked by BitField.get_prep_value and converted to 15 (0xf), the test code uses a direct SQL statement. Its implementation has a few peculiarities that may be undesirable:

  • It uses an Django internal API, the Field.column attribute. Granted, this package already uses a lot of internal APIs, and Field.column is highly unlikely to change. However, in general using less internal APIs is better for future compatibility.

  • Using low-level API misses a lot of code paths that could have been tested.

  • Neither db_table nor db_column is escaped. In case we later incorporate tests involving pathological SQL object identifiers, we have to further use quote_name, which is not exactly public API either.

Instead, we use models.Value() with an explicit output_field, which still avoids BitField.get_prep_value and inserts the value directly.

Further, directly assign to __dict__ so that the BitFieldCreator descriptor's __set__ method is bypassed and the value is assigned unchanged.

@iamahuman iamahuman force-pushed the test-no-direct-sql branch 3 times, most recently from 3fc4ef5 to ded42f5 Compare January 16, 2021 14:52
@iamahuman iamahuman marked this pull request as draft January 18, 2021 13:34
@iamahuman iamahuman marked this pull request as ready for review January 18, 2021 13:38
@iamahuman iamahuman force-pushed the test-no-direct-sql branch 2 times, most recently from e46df85 to ea34e02 Compare January 18, 2021 13:53
Supersedes disqus#11.

In order to prevent (-1) from being masked by BitField.get_prep_value
and converted to 15 (0xf), the test code uses a direct SQL statement.
Its implementation has a few peculiarities that may be undesirable:

- It uses an Django internal API, the Field.column attribute.  Granted,
  this package already uses a lot of internal APIs, and Field.column
  is highly unlikely to change.  However, in general using less internal
  APIs is better for future compatibility.

- Using low-level API misses a lot of code paths that could have been
  tested.

- Neither db_table nor db_column is escaped.  In case we later
  incorporate tests involving pathological SQL object identifiers, we
  have to further use quote_name, which is not exactly public API
  either.

Instead, we use models.Value() with an explicit output_field, which
still avoids BitField.get_prep_value and inserts the value directly.

Further, directly assign to __dict__ so that the BitFieldCreator
descriptor's __set__ method is bypassed and the value is assigned
unchanged.
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.

1 participant