Improve new-password validation errors

adam-thomas commented 8 years ago

Backporting in some fixes from later versions to v8.1.x.

@incuna/backend review please?

adam-thomas commented 8 years ago

Travis is totally broken right now:

py27-django16 runtests: commands[0] | coverage run ./user_management/tests/run.py
Coverage.py warning: No data was collected.
Traceback (most recent call last):
  File "./user_management/tests/run.py", line 7, in <module>
    from colour_runner.django_runner import ColourRunnerMixin
  File "/home/travis/build/incuna/django-user-management/.tox/py27-django16/lib/python2.7/site-packages/colour_runner/django_runner.py", line 1, in <module>
    from .runner import ColourTextTestRunner
  File "/home/travis/build/incuna/django-user-management/.tox/py27-django16/lib/python2.7/site-packages/colour_runner/runner.py", line 3, in <module>
    from .result import ColourTextTestResult
  File "/home/travis/build/incuna/django-user-management/.tox/py27-django16/lib/python2.7/site-packages/colour_runner/result.py", line 16, in <module>
    class ColourTextTestResult(result.TestResult):
  File "/home/travis/build/incuna/django-user-management/.tox/py27-django16/lib/python2.7/site-packages/colour_runner/result.py", line 28, in ColourTextTestResult
    _terminal = Terminal()
  File "/home/travis/build/incuna/django-user-management/.tox/py27-django16/lib/python2.7/site-packages/blessings/__init__.py", line 105, in __init__
    self._init_descriptor)
error: setupterm: could not find terminal
ERROR: InvocationError: '/home/travis/build/incuna/django-user-management/.tox/py27-django16/bin/coverage run ./user_management/tests/run.py'
___________________________________ summary ____________________________________
ERROR:   py27-django16: commands failed
The command "tox -e $TOX_ENV" exited with 1.

In the meantime, could I get some review? :P

kevinetienne commented 8 years ago

I think the error is related to https://github.com/incuna/django-user-management/pull/138 looks like @Ian-Foote suggested a fix but we had another error.

adam-thomas commented 8 years ago

I have also seen ValueError: I/O operation on closed file. locally, I assumed it was something to do with my configuration. I guess once Travis gets its stuff together we'll see that as well, in that case. Any suggestions on what to do about either error?

kevinetienne commented 8 years ago

I will try to not use a constant for SIMPLE_PNG for the second error. Or to find maybe if this behavior has been introduced with a new version of PIL.

adam-thomas commented 8 years ago

@KevinEtienne I fixed the simple_png thing.

adam-thomas commented 8 years ago

@incuna/backend Travis is still failing. It's the same insta-failure as before. Any ideas? :/

adam-thomas commented 8 years ago

(The tests do pass locally, if people want to merge.)

meshy commented 8 years ago

It should be reasonably easy to fix the tests.

adam-thomas commented 8 years ago

How bizarre. Thanks!

meshy commented 8 years ago

I know -- super weird.

adam-thomas commented 8 years ago

Er, wait. I appear to already have that line in my .travis.yml.

adam-thomas commented 8 years ago

https://github.com/incuna/django-user-management/blob/serializer-validation/.travis.yml#L5

meshy commented 8 years ago

Ah.

Poop.

@incuna/backend (and @jturnbull) any ideas on how to fix the tests here?

adam-thomas commented 8 years ago

There is one other difference in @jturnbull's PR's travis.yml, so I've added it here.

adam-thomas commented 8 years ago

https://travis-ci.org/incuna/django-user-management/jobs/83073179#L138 OK we have actual errors now. Apparently one of these changes doesn't work on Django 1.6. :(

meshy commented 8 years ago

adam-thomas commented 8 years ago

Woohoo, it works now!

jturnbull commented 8 years ago

Adam, once merged, can you merge this into #138 as one release please

adam-thomas commented 8 years ago

Sure!

meshy commented 8 years ago

Don't forget the changelog!

adam-thomas commented 8 years ago

Change is logged!

meshy commented 8 years ago

If you're merging this with #138, you ought to use the same version number it dictates

adam-thomas commented 8 years ago

I can do that when I merge it with #138 tbf?

adam-thomas commented 8 years ago

doesn't matter, did it anyway :)

adam-thomas commented 8 years ago

Bump?

adam-thomas commented 8 years ago

Continued, firmer bumpage.

adam-thomas commented 8 years ago

Thanks @jturnbull :)

jturnbull commented 8 years ago

Thanks adam, sorry for ignoring you :D

adam-thomas commented 8 years ago

hehehe, appreciated :P

coveralls commented 7 years ago

Changes Unknown when pulling 0baea49f9e995020029e36349bbd25aa633503ca on serializer-validation into on v8.1.x.

incuna / django-user-management

Improve new-password validation errors #141