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

Try to fix non-determinism in tests #856

Merged
merged 2 commits into from
Oct 25, 2016
Merged

Conversation

ian-ross
Copy link
Contributor

Proposed changes in this pull request

Add ordering for resources to make database queries in tests deterministic.

When should this PR be merged

Right away, if it works.

Risks

Has a database migration, but it's completely innocuous.

Follow up actions

None.

@@ -47,6 +47,9 @@ class Resource(RandomIDModel):

history = HistoricalRecords()

class Meta:
ordering = ('name',)
Copy link
Contributor

Choose a reason for hiding this comment

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

Resources could have the same name so we still have a nondeterministic ordering. It may help to have the resource ID as the second field.

On the other hand, this ordering is simply to make testing deterministic. So if all our tests stick to unique resource names, then this should be OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. It's just for testing.

@@ -0,0 +1,23 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't we agree that: "No more auto migrations, please!"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crap. You're right. I'll rename it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was just trying to do a quick and dirty fix...

@oliverroick oliverroick merged commit 40e134d into master Oct 25, 2016
@oliverroick oliverroick deleted the bugfix/test-nondeterminism branch October 25, 2016 08:54
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.

3 participants