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.

Sign up for GitHub

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

Jump to bottom

Fixed typo in migration optimizer test. #10878

Merged
merged 1 commit into from Jan 21, 2019
Merged

Fixed typo in migration optimizer test. #10878

merged 1 commit into from Jan 21, 2019

Conversation

vytisb
Copy link
Contributor

@vytisb vytisb commented Jan 20, 2019

Introduced in a97845a.

@jdufresne
Copy link
Member

Do you know why this test didn't fail before? Seems like a typo of this kind should result in a failure.

@vytisb
Copy link
Contributor Author

vytisb commented Jan 21, 2019

assertOptimizesTo checks operation attributes but doesn't get into field attributes.

I have improved assertOptimizesTo for another ticket I'm working on. Should I add that improvement here?

@timgraham
Copy link
Member

Yes, you could add that commit here.

@vytisb
Improved assertion rigorousness in migration optimizer tests.
f11952c 
Comparing deconstructed operations doesn't check field attributes. A recursive deconstruction is needed.
@vytisb
Copy link
Contributor Author

vytisb commented Jan 21, 2019

I have added the assertOptimizesTo modiication. No idea why the build failed...

@timgraham timgraham merged commit d02b2aa into django:master Jan 21, 2019
@vytisb vytisb deleted the migration_optimizer_typo branch January 21, 2019 22:35
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
3 participants
@vytisb @jdufresne @timgraham