feat: [WIP] Migration to `nox` testing automation framework by mf2199 · Pull Request #466 · googleapis/python-spanner-django · GitHub
Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

feat: [WIP] Migration to nox testing automation framework#466

Closed
mf2199 wants to merge 6 commits into
googleapis:masterfrom
MaxxleLLC:nox
Closed

feat: [WIP] Migration to nox testing automation framework#466
mf2199 wants to merge 6 commits into
googleapis:masterfrom
MaxxleLLC:nox

Conversation

@mf2199

@mf2199 mf2199 commented Aug 24, 2020

Copy link
Copy Markdown
Contributor

Since nox is the primary testing automation tool used by Google API, we believe that its earliest integration with the Spanner-Django project is necessary to ensure its continuous compliance with the coding standards from the start. This is also a step towards future merging of Django-related code into the main Spanner repository and may save considerable amount of time and effort at later stages of development.

Major change list:

  1. Added new file noxfile.py [largely copied from Spanner main repo];
  2. Added missing folders:
  • docs
  • google
  • google/cloud
  • docs/_static
  1. Folders renamed/moved:
  • django_spanner --> google/cloud/spanner_django;
  • spanner_dbapi --> google/cloud/spanner_dbapi;
  • tests/spanner_dbapi --> tests/unit;
  • examples --> samples;
  1. Added relevant __init__.py and conf.py files;
  2. Added docs styling definitions, custom.css [copied from main Spanner repo];
  3. Resolved missing dependencies in setup.py;
  4. README.md converted to README.rst [fixing some formatter check errors];
  5. Added docs/index.rst file to comply with Sphinx requirements (the file is blank and to be updated as necessary);
  6. Updated .gitignore.

From the results of the initial testing, the code has numerous assertion errors and coverage gaps, see the test log below, which have little to do with the test setup as such. Therefore, addressing those will be a subject of subsequent PRs, if and when the matters of the current proposition are reviewed and agreed upon.

Towards #474.

@google-cla google-cla Bot added the cla: yes This human has signed the Contributor License Agreement. label Aug 24, 2020
@mf2199 mf2199 requested a review from c24t August 24, 2020 15:34
@mf2199

mf2199 commented Aug 24, 2020

Copy link
Copy Markdown
Contributor Author

@mf2199 mf2199 added api: spanner Issues related to the googleapis/python-spanner-django API. type: process A process-related concern. May include testing, release, or the like. labels Aug 24, 2020

@c24t c24t left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of unrelated changes for one PR, and it looks to me like copying the noxfile straight from python-spanner will cause more harm than good here.

Since the formatting changes are mixed in with code and config changes, this will make for a difficult git history too. Better to quarantine the formatting changes in their own PR, maybe at the same time as the change to the linter/formatter config that makes these changes required.

I think we'll save a lot of time by starting with working config and building up to the same (or similar) set of targets as in python-spanner instead of checking in a broken config and then working backwards.

Here's what I'd recommend:

  1. Switch from tox to nox, with the same set of targets as in tox.ini now
  2. Add the docs target and make the changes required to get the docs to build
  3. Add test targets for other versions of python and django, these are sure to fail right now without any test changes
  4. Add the lint target and make automated formatting changes via black
  5. Add the coverage target
  6. Add system/integration tests, then add the system target

Also, why this change?

tests/spanner_dbapi --> tests/unit

Splitting the tests by module makes sense to me, even if spanner_dbapi is the only module with tests at the moment. Also all tests so far are unit tests, I don't see the argument for a separate tests/unit/ dir.

And this one:

django_spanner --> google/cloud/spanner_django

Should this be in the google.cloud namespace package? Maybe it should! But I don't know that the change should be part of this PR.

@c24t

c24t commented Sep 10, 2020

Copy link
Copy Markdown
Contributor

@c24t c24t closed this Sep 10, 2020
@IlyaFaer IlyaFaer deleted the nox branch February 1, 2021 08:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner-django API. cla: yes This human has signed the Contributor License Agreement. type: process A process-related concern. May include testing, release, or the like.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants