-
-
Notifications
You must be signed in to change notification settings - Fork 14
Applying tdd on select-only exercise #219
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
Conversation
|
I'm not a fan of the multi-file approach. If we introduce |
This reverts commit 93bc000.
|
The latest version has the user enter all {
"version": 3,
"status": "fail",
"tests": [
{
"description": "ALL records => SELECT * FROM weather_readings",
"status": "pass"
},
{
"description": "Just location and temperature columns => SELECT location, temperature FROM weather_readings",
"status": "pass"
},
{
"description": "Without \"FROM\" => SELECT 'Hello, world.'",
"status": "fail",
"message": "Expected: [{\"'Hello, world.'\": \"Hello, world.\"}]",
"output": "Actual: [{\"say_hi\": \"Hello, world.\"}]"
},
{
"description": "All records from Seatle location => SELECT * FROM weather_readings WHERE location = 'Seattle'",
"status": "pass"
},
{
"description": "All records where humidity in range => SELECT * FROM weather_readings WHERE humidity BETWEEN 60 AND 70",
"status": "pass"
},
{
"description": "Just location column => SELECT location FROM weather_readings",
"status": "pass"
},
{
"description": "Only unique locations => SELECT DISTINCT location FROM weather_readings",
"status": "pass"
}
]
} |
| {"date":"2025-10-22","location":"Seattle","temperature":56.2,"humidity":66}, | ||
| {"date":"2025-10-22","location":"Boise","temperature":60.4,"humidity":55}, | ||
| {"date":"2025-10-23","location":"Portland","temperature":54.6,"humidity":70}, | ||
| {"date":"2025-10-23","location":"Seattle","temperature":57.79999999999999,"humidity":68}, |
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.
For some reason, this comes out as 57.79999999999 instead of the original 57.8 in the input data (see data.csv). Any ideas from the SQLite gurus on how to make it come out as 57.8?
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.
Floating point numbers are hard for computers.
SQLite promises to preserve the 15 most significant digits of a floating point value.
ref
Numbers like 57.8 (or 0.8) cannot actually be handled very well by computers using the common representations. The original only looks like 57.8 when input or rounded. Float comparisons generally go better when you compare that the two values are close enough.
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.
You can bypass this problem by picking nicer values, eg fractions which can be represented as a / (2^b).
sqlite> .mode json
sqlite> SELECT 57.8;
[{"57.8":57.79999999999999716}]
sqlite> SELECT 57.5;
[{"57.5":57.5}]
|
Adding a Python dependency to the test runner makes the image much, much larger. Without Python, I believe much of what you're doing in Python can be done pretty easily using Using the # out
[{"date":"2025-10-22","location":"Portland","temperature":53.1,"humidity":72},{"date":"2025-10-22","location":"Seattle","temperature":56.2,"humidity":66},{"date":"2025-10-22","location":"Boise","temperature":60.4,"humidity":55},{"date":"2025-10-23","location":"Portland","temperature":54.6,"humidity":70},{"date":"2025-10-23","location":"Seattle","temperature":57.79999999999999,"humidity":68},{"date":"2025-10-23","location":"Boise","temperature":62.0,"humidity":58}]
[{"location":"Portland","temperature":53.1},{"location":"Seattle","temperature":56.2},{"location":"Boise","temperature":60.4},{"location":"Portland","temperature":54.6},{"location":"Seattle","temperature":57.79999999999999},{"location":"Boise","temperature":62.0}]
[{"'Hello world.'":"Hello, world."}]
[{"date":"2025-10-22","location":"Seattle","temperature":56.2,"humidity":66},{"date":"2025-10-23","location":"Seattle","temperature":57.79999999999999,"humidity":68}]
[{"date":"2025-10-22","location":"Seattle","temperature":56.2,"humidity":66},{"date":"2025-10-23","location":"Portland","temperature":54.6,"humidity":70},{"date":"2025-10-23","location":"Seattle","temperature":57.79999999999999,"humidity":68}]
[{"location":"Portland"},{"location":"Seattle"},{"location":"Boise"},{"location":"Portland"},{"location":"Seattle"},{"location":"Boise"}]
[{"location":"Portland"},{"location":"Seattle"},{"location":"Boise"}]# test.jq
def range(upto):
def _range:
if . < upto then ., ((.+1)|_range) else empty end;
0 | _range;
[
range($want[0]|length) as $number
| $want[0] | .[$number | tonumber].expected as $expected
| $want[0] | .[$number | tonumber].description as $description
| $out|.[$number | tonumber] as $got
| if $got != $expected then {$description, $expected, $got, "status": "fail"} else {$description, "status": "pass"} end
]» jq -c --slurpfile out out --slurpfile want test_data.json -n -f test.jq 2>&1
[
{"description": "ALL records => SELECT * FROM weather_readings", "status": "pass"},
{"description": "Just location and temperature columns => SELECT location, temperature FROM weather_readings", "status": "pass"},
{
"description": "Without \"FROM\" => SELECT 'Hello, world.'",
"expected": [{"'Hello, world.'": "Hello, world."}],
"got": [{"'Hello world.'": "Hello, world."}],
"status": "fail"
},
{"description": "All records from Seatle location => SELECT * FROM weather_readings WHERE location = 'Seattle'", "status": "pass"},
{"description": "All records where humidity in range => SELECT * FROM weather_readings WHERE humidity BETWEEN 60 AND 70", "status": "pass"},
{"description": "Just location column => SELECT location FROM weather_readings", "status": "pass"},
{"description": "Only unique locations => SELECT DISTINCT location FROM weather_readings", "status": "pass"}
]My main worry here is with lining up the various outputs vs tests in a list. It's not obvious that the I think this might be viable, but we'd also need some additional guardrails.
-- Do not remove the comments! The comments line up with the test outputs.
-- Write your solution to each task beneath the comment.
-- 1: Select all records.
-- 2: Select the location and temperature.
-- 3: Say Hello!
... |
|
@IsaacG, thanks for the jq prototype. I think it looks promising! I will use your code as a starting point, and work on adding the guardrails you mentioned.
I agree, and I think the numbers should correspond to individual tasks within the exercise, like the tasks in exercises such as the Role Playing Game in the Elm track (among many others). |
- Split exercise into separate tasks - Capture actual output of each task in a separate table - Store expected output for each task in a separate table
|
Here's my latest iteration, which attempts to correspond with the latest ideas discussed in the forum thread. The major TODO at this point is to generate test results in a way that will work effectively. Any ideas from the reviewers on how to do this? |
|
I agree; the Python file is no longer used -- I just forgot to remove it.
OK, I'll give it a try... |
- Use only INTEGER data, for simplicity - Add shell script to generate report - Add jq filters to aid in report generation - Store test data in json file, not in db
|
@IsaacG: here is my latest iteration. The new version relies on a shell script to generate the report, which invokes two different jq filters to render It's a little convoluted: One TODO item: improve the readability of the message on failures. The new version generates the following How does this grab you? |
|
I think this has good potential. The message is a bit messy, but overall I think this approach can work. Why does the Hello World expected/actual have the string repeated? Would it make sense to format the message using CSV? |
|
Does this approach hide the fact that the |
I'm glad you like it. I agree that the message formatting could use some improvement. I will plan to spend some time improving it in this draft PR. After that, I'm thinking about adding an exercise following this approach to #217. How does that sound?
In general, the tests must ensure that both the column names and the data match expectations. Hello World is a special case; I would not include it in a real exercise since there is already a Hello World exercise. |
The exercise instructions would specify the required columns and describe the expected data. Each test would compare both the column names and the data against the expected result. |
|
@IsaacG, any ideas on how to correlate user output with the specific test(s) in which it was generated? One way is to have an |
Also: - Conform to test runner interface, filenames - Capture user output - Cause SQL test script to exit with error on test failure - Handle empty output from test - Remove "Hello, world" test
|
@IsaacG - here is the latest draft version, which includes the following:
Running the current version generates the |
I'm not following. Doesn't each test use a different table? |
By "user output" I mean the captured stdout content that goes into the "output" field in Anyway, after testing the latest version with the test runner, I now realize that |
- Aggregate only individual test results in output.json - Don't change exit code based on test result
|
@IsaacG I successfully tested the latest version with the test runner. I have pasted the resulting content in |
glennj
left a comment
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.
Just a few details, but overall it's a great approach.
| | $single_test_data.expected as $expected | ||
| | $single_test_data.task_id as $task_id | ||
| | if $task_id then {$task_id} else {} end as $entry | ||
| | $entry + {$description} as $entry |
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.
| | $entry + {$description} as $entry | |
| | $entry += {$description} |
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.
Actually, this suggestion didn't work as expected -- it did not modify $entry. So I will revert this one. But please let me know if you have other ideas on how to simplify it!
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.
Right, careless of me. How about this: what you're trying to do is select some details from the slug object in the test_data and add the status.
Can we just use that object directly?
$test_data[0][$slug]
| if $got == .expected then
. + { "status": "pass" }
else
. + { "status": "fail",
"message": failure_message($got; .expected) }
endThere 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.
Almost. We also want to remove .expected from the result. But I think I can slightly tweak your example to do that. Thanks!
|
Very solid ideas here. Me gustan. :) |
Co-authored-by: Glenn Jackman <[email protected]> Co-authored-by: Isaac Good <[email protected]>
| # Generate result for each test | ||
| rm -f results.txt | ||
| for SLUG in "${SLUGS[@]}"; do | ||
| ACTUAL=$(sqlite3 -json $DB_FILE "SELECT * FROM ${SLUG};" | tr -d '[:space:]') |
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.
Why would there be spaces in the slags?
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.
There wouldn't be spaces in the slugs, but there would be in the SELECT results. Maybe table would be a better variable name than SLUG in this context.
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.
... or maybe not, since it is used as the slug on line 11.
Co-authored-by: Isaac Good <[email protected]>
| .mode markdown | ||
| .output user_output.md | ||
| .read ./intro-select.sql | ||
| .shell rm -f ./results.db |
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.
Shoudl the rm happen before the read?
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.
I don't think so. If you're worried about a pre-existing results.db, I have confirmed that the .save command will overwrite such.
|
|
||
| # Generate result for each test | ||
| for slug in "${slugs[@]}"; do | ||
| actual=$(sqlite3 -json $db_file "SELECT * FROM ${slug};" | tr -d '[:space:]') |
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.
Why are you removing spaces from the select?
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.
Probably leftover from an earlier version that was doing a textual comparison. Will remove the tr.
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.
Done.
| {"location":"Boise"} | ||
| ] | ||
| } | ||
| } No newline at end of file |
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.
Trailing newline please
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.
Done.
| "task_id": 1, | ||
| "description": "All data", | ||
| "expected": [ | ||
| {"date":"2025-10-22","location":"Portland","temperature":53,"humidity":72}, |
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.
Is there a reason there's no spaces after , and : in part of the data?
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.
No reason. Will add spaces.
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.
Fixed.
An alternative to #218, which stores the results of each SELECT statement in a separate table.