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

Use pytest for tests? #50

Open
blueyed opened this issue Jan 17, 2015 · 18 comments
Open

Use pytest for tests? #50

blueyed opened this issue Jan 17, 2015 · 18 comments

Comments

@blueyed
Copy link
Contributor

blueyed commented Jan 17, 2015

The current runtests.py script does not work for Django 1.7 (or 1.8?) anymore, and while at it I've wondered what's your opinion on using pytest for tests?

@mauricioabreu
Copy link
Contributor

I don't know what is his opinion but I would put my efforts to make it happen.

@blueyed
Copy link
Contributor Author

blueyed commented Feb 6, 2015

btw: there will be a "Adopt pytest month" probably:
https://mail.python.org/pipermail/pytest-dev/2015-February/002763.html

@mauricioabreu
Copy link
Contributor

Yeah, it is a good time to do it but I don't if it is safe to contribute to this project. Author is missing so he is not even giving any feedback on pull requests and issues.

@blueyed
Copy link
Contributor Author

blueyed commented Feb 7, 2015

@mauricioabreu
We could always create a (temporary) fork with our improvements / fixes.

But you're right about the unresponsiveness from the @agiliq team.

@akshar-raaj
Copy link
Member

@blueyed @mauricioabreu Please accept my apologies about our unresponsiveness. Looking into your older comments, please feel free to send a PR. We plan to maintain this app so your PR will not go unnoticed.

@blueyed
Copy link
Contributor Author

blueyed commented Feb 8, 2015

@akshar-raaj
This is good to hear. Thanks for letting us know.

@mauricioabreu
Copy link
Contributor

hey @akshar-raaj of course I accept your apologies. :)
That is not your fault or anyone's else. We are just trying to make this app better and better.

I am currently migrating runtests.py to pytest (conftest). Actually only one test is failing using Django 1.7.x
After figuring out the problem I will change how tests are written. Should I do it step by step?

@blueyed
Copy link
Contributor Author

blueyed commented Feb 9, 2015

@mauricioabreu
Great. I think it would be great to push a separate branch for the pytest integration.

I would say that for the beginning existing tests should not get rewritten, but should work with the Django testrunner and pytest. But then new tests might be written using pytest features, if it gets accepted/merged.

@blueyed
Copy link
Contributor Author

blueyed commented Feb 9, 2015

Re Django 1.7:
The failing test for Django 1.7 is "test_radio_select (parsley.tests.tests.TestRadioSelect)".

Please see #49 for a WIP branch to get Django 1.7 support.

@mauricioabreu
Copy link
Contributor

@blueyed Agreed.
I think your pull request could be merged. Django 1.7 is a important thing.

@mauricioabreu
Copy link
Contributor

Hey @blueyed now I see what you said.
Yes, I had the same error. How can we proceed? Get your PR merged, so we can at least have the test suite using Django 1.7 and after that create a branch pytest (like I did but did not push yet) and keep coding there?

@blueyed
Copy link
Contributor Author

blueyed commented Feb 9, 2015

@mauricioabreu
The patch to runtests.py is quite small, and the rest from the django-1.7 branch (currently) is for travis only.

You could create the pytest-branch from master, and then cherry-pick just blueyed@e1c1b5a.
Or branch it off the PR's branch, but that might get rebased later and not merged as-is.
But then it would still be possible to get commits from the pytest branch into master / a PR.

But yes, merging the PR #49 already would simplify things.

@akshar-raaj
Can this be done in the next days?

@mauricioabreu
Copy link
Contributor

I got it @blueyed
Let's wait for the owners decision.

@blueyed
Copy link
Contributor Author

blueyed commented Mar 25, 2015

@akshar-raaj
ping?

@mauricioabreu
Copy link
Contributor

:(

@akshar-raaj
Copy link
Member

@shivakrshn49 I see you have merged this PR. Is the issue done and can we close it?

@jproffitt
Copy link
Collaborator

This issue seems to be resolved, yes?

@blueyed
Copy link
Contributor Author

blueyed commented Dec 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants