8000 test: run Django tests against Spanner emulator by IlyaFaer · Pull Request #559 · googleapis/python-spanner-django · GitHub
[go: up one dir, main page]
Skip to content

test: run Django tests against Spanner emulator #559

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 33 commits into from
Closed

test: run Django tests against Spanner emulator #559

wants to merge 33 commits into from

Conversation

IlyaFaer
Copy link
Contributor

No description provided.

@IlyaFaer IlyaFaer added type: process A process-related concern. May include testing, release, or the like. api: spanner Issues related to the googleapis/python-spanner-django API. labels Nov 30, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 30, 2020
IlyaFaer
IlyaFaer commented Nov 30, 2020
View reviewed changes
Copy link
Contributor Author
@IlyaFaer IlyaFaer left a comment

Choose a reason for hiding this comment

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

Now we can drop parallelize_tests.go, its binary version, build.sh and several environment variables. I'm still keeping them in here, until all the things fixed.

As for the transaction commited error, I've pushed a PR with fix in the original Spanner repo:
googleapis/python-spanner#177

c24t
c24t reviewed Dec 1, 2020
View reviewed changes
Copy link
Contributor
@c24t c24t left a comment

Choose a reason for hiding this comment

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

A few comments, but looks like a big improvement over running the django tests against non-emulated spanner.

I'd like to see the tests passing before we merge this, but it looks like that depends on googleapis/python-spanner#177. Did you check that the tests pass using your own fork of python-spanner with this change applied?

@IlyaFaer IlyaFaer changed the title test: run Django tests agains Spanner emulator test: run Django tests against Spanner emulator Dec 1, 2020
@FirePing32 FirePing32 mentioned this pull request Dec 1, 2020
Closed
@IlyaFaer IlyaFaer added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 3, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2020
@IlyaFaer
Copy link
Contributor Author
IlyaFaer commented Dec 3, 2020 •
edited
Loading

@c24t, now we have some Django tests failing because of several problems. DELETE must have WHERE clause error isn't dependent on adding dummy WHERE clause (I've already tried to revert it), so it requires some digging. Working on it.

@IlyaFaer IlyaFaer added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 4, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 4, 2020
@IlyaFaer
Copy link
Contributor Author
IlyaFaer commented Dec 4, 2020

@c24t, I've got a green light on Django tests. There are still some things to refactor and improve, but I suppose it's not something to block the release.

@IlyaFaer IlyaFaer marked this pull request as ready for review December 4, 2020 10:40
@IlyaFaer IlyaFaer requested a review from a team as a code owner December 4, 2020 10:40
@IlyaFaer
Copy link
Contributor Author
IlyaFaer commented Dec 7, 2020 •
edited
Loading

@c24t, okay, I've found the place where the basic tests were set to be executed, and fixed it.

But I'm little bit stuck with hitting 950 query args quota. Not sure why this error showed yet (maybe my eyes just soaped, but), everything seems to be set correctly: skips, dependencies, tests things like max_query_params.

UPDATE: too many authorisation rules seems to be prepared for tests (they are inserted within a huge bunch into a table) - that's the problem.

@IlyaFaer IlyaFaer force-pushed the dj_tests_against_emulator branch from 5c104e6 to 4a23973 Compare December 7, 2020 12:07
@IlyaFaer
Copy link
Contributor Author
IlyaFaer commented Dec 8, 2020 •
edited
Loading

@c24t, so, the Django tests on emulator are canceled because of a timeout hit. Seems to me we still need parallelisation, as slow execution problem has not gone with the emulator.

@IlyaFaer
Copy link
Contributor Author

@c24t, okay, 8 threads seems to be working, but there is a lot of Primary key already exists error. Aren't emulators conflicting with each other? Maybe we could use separate workflows?

@IlyaFaer IlyaFaer closed this Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c24t c24t c24t left review comments

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@IlyaFaer @c24t @yoshi-kokoro
0