Skip to content

Handle spaces in DB names#590

Merged
simonw merged 2 commits intosimonw:masterfrom
rixx:issue-503-names-with-spaces
Nov 4, 2019
Merged

Handle spaces in DB names#590
simonw merged 2 commits intosimonw:masterfrom
rixx:issue-503-names-with-spaces

Conversation

@rixx
Copy link
Contributor

@rixx rixx commented Oct 11, 2019

Closes #503

@simonw
Copy link
Owner

simonw commented Oct 14, 2019

We need some unit tests for this.

I'd be fine with us renaming extra_database.db to extra database.py in these existing fixtures:

datasette/tests/fixtures.py

Lines 174 to 185 in af2e6a5

@pytest.fixture(scope="session")
def app_client_two_attached_databases():
yield from make_app_client(
extra_databases={"extra_database.db": EXTRA_DATABASE_SQL}
)
@pytest.fixture(scope="session")
def app_client_two_attached_databases_one_immutable():
yield from make_app_client(
is_immutable=True, extra_databases={"extra_database.db": EXTRA_DATABASE_SQL}
)

Then we could add an extra test_database_name_with_spaces(...) test in test_api.py which uses parametrize to exercise a bunch of URLs and check they all return a 200 - something a bit like this but covering a range of URL paths (one for database, table, row and table with filters):

datasette/tests/test_api.py

Lines 746 to 765 in 90d4f49

@pytest.mark.parametrize(
"path,expected_rows,expected_pages",
[
("/fixtures/no_primary_key.json", 201, 5),
("/fixtures/paginated_view.json", 201, 5),
("/fixtures/no_primary_key.json?_size=25", 201, 9),
("/fixtures/paginated_view.json?_size=25", 201, 9),
("/fixtures/paginated_view.json?_size=max", 201, 3),
("/fixtures/123_starts_with_digits.json", 0, 1),
# Ensure faceting doesn't break pagination:
("/fixtures/compound_three_primary_keys.json?_facet=pk1", 1001, 21),
# Paginating while sorted by an expanded foreign key should work
(
"/fixtures/roadside_attraction_characteristics.json?_size=2&_sort=attraction_id&_labels=on",
5,
3,
),
],
)
def test_paginate_tables_and_views(app_client, path, expected_rows, expected_pages):

@rixx
Copy link
Contributor Author

rixx commented Oct 14, 2019

Ah, thank you – I saw the need for unit tests but wasn't sure what the best way to add one would be.

@rixx rixx force-pushed the issue-503-names-with-spaces branch from 9ba6d67 to 34ee816 Compare October 14, 2019 09:56
@rixx rixx force-pushed the issue-503-names-with-spaces branch from 34ee816 to 9d54ca9 Compare October 14, 2019 09:57
@rixx
Copy link
Contributor Author

rixx commented Oct 14, 2019

Added tests.

@simonw simonw merged commit 931bfc6 into simonw:master Nov 4, 2019
simonw pushed a commit that referenced this pull request Nov 11, 2019
@rixx @simonw
Handle spaces in DB names (#590)
e5dc89a 
Closes #503 - thanks, @rixx
simonw pushed a commit that referenced this pull request Nov 15, 2019
@rixx @simonw
Handle spaces in DB names (#590)
c43e113 
Closes #503 - thanks, @rixx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Handle SQLite databases with spaces in their names?

2 participants

@rixx @simonw