Skip to content
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

Add tests to verify db version hasn't changed #4688

Merged
merged 1 commit into from
Dec 20, 2014

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Dec 20, 2014

issue #4479

  • these tests will need updating if ever the db schema (not inc validations), permissions fixtures or db
    version ever changes
  • the intention is that it is no longer possible to accidentally change permissions or the db schema without
    knowing you need to update the db version and the values in this test file

issue TryGhost#4479

- these tests will need updating if ever the db schema (not inc validations), permissions fixtures or db
version ever changes
- the intention is that it is no longer possible to accidentally change permissions or the db schema without
knowing you need to update the db version and the values in this test file
@ErisDS
Copy link
Member Author

ErisDS commented Dec 20, 2014

I did this as a test as I figured it was easier and cleaner than adding more weird validations to core. This check is only intended to give developers a hint that they've changed something important - and to make it easier to review PRs, so as it's dev-only, I think a test makes sense?

@jaswilli
Copy link
Contributor

I like the idea of watching changes in these files. Two concerns:

a) Re-generating the hashes is non-trivial, which isn't all bad, but there's a middle ground between it being too easy where you don't have to stop and think about what you're doing, and it being punitive.

b) I'm wondering how portable the transformations of the data are that occur before generating the hash to test? Do the lodash methods and JSON.stringify produce byte perfect output across all versions of node, on all platforms, on all locales?

All that being said, I'm not against putting this in and just seeing how it goes since it's just a test and doesn't really tie us to it long term.

@ErisDS
Copy link
Member Author

ErisDS commented Dec 20, 2014

Regenerating the hashes shouldn't be too tricky, just console log the values from line 39 and 40 on line 41 and copy and paste them back in? But actually, the test should output the incorrect value when it fails, so I figured it was easy enough as generating the hash is also managed by running the tests.

I am not sure about the lodash and stringify methods but as you say I think it's easy enough to add a skip if this doesn't work out 😉

jaswilli added a commit that referenced this pull request Dec 20, 2014
Add tests to verify db version hasn't changed
@jaswilli jaswilli merged commit b002798 into TryGhost:master Dec 20, 2014
@ErisDS ErisDS deleted the migrate-test branch February 28, 2015 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants