Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Just write a test for it

kobzol.github.io

Post reply on HN
Print view View original

51–60 of 62 posts

Re: Just write a test for it

#51

Post by flqn »

dreghgh wrote: post #13

This is the wrong approach. He should be putting data in the database schema before trying to run migrations on it. Doing that is simple and can potentially catch all sorts of bugs, including the bugs you didn't think of yet. His solution is complicated but only catches one very specific type of bug.

That would certainly catch nearly all migration issues, but it doesn't provide a helpful error message like a test for a specific mistake like this does.

Ideally both approaches would be used, with the general case being used to detect and inform more targeted tests.

Re: Just write a test for it

#53

Post by dreghgh »

dxdm wrote: post #39
dreghgh wrote: post #30

Earlier quoted context omitted.

You should solve the problem you need to solve, not the problem you have a cool solution for. The problem to solve is "given my database in an arbitrary but valid state, applying a migration should succeed and leave the database in a equally valid state". Not "how do I stop people adding NOT NULL columns into tables".

You're just over-generalizing the problem into something you will not be able to solve by proving that a migration works for a _specific_ arbitrary state.

I mean this is a widely adopted practice so it clearly is not impossible to solve.

All tests substitute small collections of representative inputs for the generality of inputs.

Re: Just write a test for it

#54

Post by dxdm »

dreghgh wrote: post #53
dxdm wrote: post #39

Earlier quoted context omitted.

You're just over-generalizing the problem into something you will not be able to solve by proving that a migration works for a _specific_ arbitrary state.

I mean this is a widely adopted practice so it clearly is not impossible to solve. All tests substitute small collections of representative inputs for the generality of inputs.

Of course, but then let's not knock the OP article for covering a different subset of testable states, in a way that will need no adjusting for future schema changes, while still preventing the actual problem.

Re: Just write a test for it

#55

Post by karparov »

wesselbindt wrote: post #44
karparov wrote: post #23

Earlier quoted context omitted.

Pre-populate the db.

This section: > Apart from parsing the SQL query, I also considered an alternative testing approach that I might implement in the future: go through each migration one by one, and insert some dummy data into the database before applying it, to make sure that we test each migration being applied on a non-empty database. The data would either have to be generated automatically based on the current database schema, or w…

It takes more than a few minutes, yes. But this one-time investment will prevent the next 10 migration-related bugs that he'll otherwise blog about.

Grab some representative data from production and keep feeding that into your migration tests. Keep updating those. Worth each minute if you care about quality.

Re: Just write a test for it

#56

Post by karparov »

nkrisc wrote: post #34
karparov wrote: post #22

Earlier quoted context omitted.

Tbf, it's not their fault it made it to the HN front page. Are we going to criticise every little innocent blog post just because somebody liked it, submitted it to HN and it got enough upvotes?

If this isn’t a place to discuss the submissions, what is it?

Discussing is obviously fine.

But sometimes some innocent blog posts get criticised as if they claimed that they solved world hunger. They don't. They are often just some random thought in a rarely-read blog. Nobody intended to go convince a gang of seasoned hackers that they have undisputable wisdom.

Re: Just write a test for it

#57

Post by davidgomes »

conradludgate wrote: post #36

I'm obviously biased by being an employee, but this is where Neon's branching[0] functionality can come in useful. We hope to expand on it one day and build more first-party migration tooling but you can already get a good enough system with the features we have. Neon Branches are zero-copy snapshots of the database, with all the same data, on an isolated postgres instance. You can run migrations on that data without…

> If performance matters, some migrations might hide table locks which can cause major slowdowns.

Do you mean that the migration might work on a side branch, but then it might not work on the main branch, because there's no other activity running on the side branch?

Re: Just write a test for it

#58

Post by polishdude20 »

karparov wrote: post #55
wesselbindt wrote: post #44

Earlier quoted context omitted.

This section: > Apart from parsing the SQL query, I also considered an alternative testing approach that I might implement in the future: go through each migration one by one, and insert some dummy data into the database before applying it, to make sure that we test each migration being applied on a non-empty database. The data would either have to be generated automatically based on the current database schema, or w…

It takes more than a few minutes, yes. But this one-time investment will prevent the next 10 migration-related bugs that he'll otherwise blog about. Grab some representative data from production and keep feeding that into your migration tests. Keep updating those. Worth each minute if you care about quality.

You'd also have to remember to update your PII removal code every time a new PII field is added!

Re: Just write a test for it

#59

Post by kobzol »

NoahZuniga wrote: post #52
switch007 wrote: post #7

Related https://github.com/sbdchd/squawk "linter for PostgreSQL, focused on migrations" It covers this class of problem I think

After reading the article I was left with the question why didn't he just use an SQL linter?

That didn't even occur to me, tbh :) But it doesn't have to be SQL linting, I just wanted to appreciate the mindset of not being lazy/afraid to write an unorthodox test.

Re: Just write a test for it

#60

Post by null_deref »

simonw wrote: post #38
null_deref wrote: post #37

Enjoyed the article. Am I out of touch or have the article linked to a PR of the rust-lang eco system that didn’t go through a CR, is this really the standard for such a large language standard library?

You mean this one? https://github.com/rust-lang/bors/pull/251 If you look at https://github.com/rust-lang/bors it's not a standard library package, it's a tool to support the Rust development process. And the person who opened and landed that PR is the lead developer of that project. Skipping a code review from someone else feels OK to me for that.

Thanks for the clarification, definitely makes sense
Post reply on HN