GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-10572 : Move sqlite3 tests to Lib/test/test_sqlite3 by erlend-aasland · Pull Request #24148 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Jan 7, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue10572

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Erlend E. Aasland added 6 commits January 7, 2021 01:41
Erlend E. Aasland
Move test directory and rename files
49f4728
* git rm __init__.py
* git mv Lib/sqlite3/test => Lib/test/test_sqlite3
* git mv test_sqlite3.py => __init__.py
* add test_ prefix to test files
Erlend E. Aasland
Add __main__.py
d2c3963
Erlend E. Aasland
Automatically load tests instead of using a hard coded list
1d2f5eb
Erlend E. Aasland
Autoload test suites instead of hard coding them
f548741
Erlend E. Aasland
Add NEWS
a2599bb
Erlend E. Aasland
Update Makefile
fdc1ff2
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jan 7, 2021
@bedevere-bot bedevere-bot added the awaiting review label Jan 7, 2021
@erlend-aasland

erlend-aasland commented Jan 7, 2021

Copy link
Copy Markdown
Contributor Author

I know you find this change controversial, @berkerpeksag, but I think it'll be nice in the end. Motivations:

  • https://bugs.python.org/issue10572#msg179596
  • test suites are simplified and less error prone (for example forgetting to include a test suite)
  • it'll be possible to do python3 Lib/test/test_sqlite3/test_types.py, for example
  • no special treatment in Makefile

What do you think?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland

erlend-aasland commented Jan 7, 2021

Copy link
Copy Markdown
Contributor Author

BTW, history is preserved by git mv.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

berkerpeksag
berkerpeksag reviewed Jan 7, 2021
View reviewed changes

@berkerpeksag berkerpeksag left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

https://bugs.python.org/issue10572#msg179596

Unfortunately, I don't find Brett's arguments convincing. Linux distros are making too many customizations that would make stripping tests a trivial change compared to others.

test suites are simplified and less error prone (for example forgetting to include a test suite)

I don't even remember last time when this was an issue, to be honest.

it'll be possible to do python3 Lib/test/test_sqlite3/test_types.py, for example

This is a nice UX improvement. However, sqlite3 tests take almost no time to run and it's relatively easy to disable other test cases.

no special treatment in Makefile

Well, sqlite3 is hardly the only exception there :)

Also, we already made backporting fixes to maintenance branches automatically harder by renaming the test names. Moving tests around would make auto backporting almost impossible which I would like to avoid doing.

I'm sure you can find more interesting stuff to work on :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@erlend-aasland

erlend-aasland commented Jan 7, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

test suites are simplified and less error prone (for example forgetting to include a test suite)
I don't even remember last time when this was an issue, to be honest.

Ok, but they're still simplified :) This PR removed a lot of lines.

it'll be possible to do python3 Lib/test/test_sqlite3/test_types.py, for example
This is a nice UX improvement. However, sqlite3 tests take almost no time to run and it's relatively easy to disable other test cases.

That's true

no special treatment in Makefile
Well, sqlite3 is hardly the only exception there :)

True, but it still declutters the Makefile (by one line)

Also, we already made backporting fixes to maintenance branches automatically harder by renaming the test names. Moving tests around would make auto backporting almost impossible which I would like to avoid doing.

Not impossible, but cumbersome. Anyway, it's a strong argument. However, the recent test changes will also make backporting cumbersome (for the next two releases only), which is a minor argument pro this change.

OTOH, this is no big deal for me. I'm ok with letting it go for now. It was a nice exercise, though :)

I'm sure you can find more interesting stuff to work on :)

I'm twiddling thumbs, waiting for #24135 ;)

Yes, there's a lot of stuff to work on. After the AC transition, I've got PR's for module state and multi-phase init. When we're there, I would like to get rid of the cyclic ref hack between Cache and Connection. I had a PR for it, but for some reason it failed on Windows. I haven't spent much time figuring out that issue, though. New features can wait for now, IMO.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@berkerpeksag

berkerpeksag commented Jan 11, 2021

Copy link
Copy Markdown
Member

I had a PR for it, but for some reason it failed on Windows. I haven't spent much time figuring out that issue, though.

I'm currently on Windows and I can take a look. It may take a while though because I mainly use it for gaming and I need to set up my development environment.

I'm closing this for now. We can consider it later. Thank you for your effort! :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@berkerpeksag berkerpeksag closed this Jan 11, 2021
@erlend-aasland

erlend-aasland commented Jan 11, 2021

Copy link
Copy Markdown
Contributor Author

I had a PR for it, but for some reason it failed on Windows. I haven't spent much time figuring out that issue, though.
I'm currently on Windows and I can take a look. It may take a while though because I mainly use it for gaming and I need to set up my development environment.

Great, thanks!

We can consider it later. Thank you for your effort! :)

I sense a sliver of hope for this bpo issue ;) I'll keep the branch around

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland erlend-aasland mentioned this pull request Oct 29, 2021
bpo-10572: Move sqlite3 tests to Lib/test #29304
Merged
@voidspace voidspace mentioned this pull request Jun 20, 2022
Move test sub-packages to Lib/test #54781
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@berkerpeksag berkerpeksag berkerpeksag left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@erlend-aasland @berkerpeksag @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.