Skip to content
This repository was archived by the owner on Apr 16, 2023. It is now read-only.

Conversation

@meshy
Copy link
Owner

@meshy meshy commented Jul 27, 2015

There's a failing test related to testing the fields on Route.
In django 1.8 we seem to be getting the related names of models that were created in other test methods.

@meshy meshy mentioned this pull request Jul 27, 2015
@kevinetienne
Copy link
Collaborator

I was thinking that Django might have a method to query the non-related fields and related fields separately which might have helped here but I didn't find anything relevant here.

@meshy
Copy link
Owner Author

meshy commented Jul 28, 2015

I think that ._meta.get_all_field_names() is deprecated, so we should perhaps replace it with something in incuna-test-utils.

@meshy
Copy link
Owner Author

meshy commented Jul 28, 2015

That said, I can't help but question the use of the test now.

@LilyFirefly
Copy link
Collaborator

Perhaps field_names is appropriate here?

@meshy
Copy link
Owner Author

meshy commented Jul 28, 2015

Sounds like a plan.

@meshy
Copy link
Owner Author

meshy commented Jul 28, 2015

Alternatively, is there a better way to create the subclasses we're testing?

@meshy
Copy link
Owner Author

meshy commented Jul 28, 2015

@ian-foote field_names returned a very similar list of fields, and did include the two that are making the tests fail.

@LilyFirefly
Copy link
Collaborator

Ah, that's a pity.

The fields refer to incoming foreign keys from subclasses that appear in
other tests.
@meshy
Copy link
Owner Author

meshy commented Jul 29, 2015

I added the new fields to what the test expected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure the slug.help_text is correctly defined. Perhaps it should be using textwrap.dedent?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, I think you're right, this is odd. Really, it should just be one line, as that's how I am expecting it to appear in the admin.

#48

LilyFirefly pushed a commit that referenced this pull request Jul 30, 2015
@LilyFirefly LilyFirefly merged commit 8c5d864 into master Jul 30, 2015
@LilyFirefly LilyFirefly deleted the django-1.8 branch July 30, 2015 13:48
@meshy
Copy link
Owner Author

meshy commented Jul 30, 2015

\o/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants