Skip to content

Conversation

@penguinolog
Copy link
Collaborator

  • asyncio coroutine was not awaited, so test was false-positive (poor visible on Travis CI) Fix test.
  • FloatEdit unittests passed, incorrect output doctests not caused exception on CI
  • warnings on version number
  • warnings about deprecated setup.py usage for tests run
  • python2.7 is not able to execute tests for asyncio due to mandatory syntax changes (async def). Fix logic: 1. added isinstance check before getting significance 2. in case of using points after delimiter, need + 2 to precision 3. properly set precision to context instead of context override 4. normalize version number 5. execute tests via unittest 6. stop using python 2.7 in tests run instead of increasing complexity
Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment

from __future__ import division, print_function

VERSION = (2, 1, 3, 'dev')
__version__ = ''.join(['-.'[type(x) == int]+str(x) for x in VERSION])[1:]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

version 1.2.3-dev0 is normalized to 1.2.3.dev0

py37
py38
pypy
envlist = py3{6,7,8,9,10,11},pypy3
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

use compact form

def test_coroutine_error(self):
evl = self.evl

async def error_coro():
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this syntax is not supported on python2.
Since sunset of python2, no reason to add magic with compile


async def error_coro():
result = 1 / 0 # Simulate error in coroutine
yield result
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yield -> async generator, we need coroutine

trio

commands =
coverage run -m unittest discover -s urwid -v
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to check GitHub on separate branch before apply to pipeline

>>> e.keypress(size, '5')
>>> e.keypress(size, '1')
>>> assert e.value() == Decimal("51.51")
>>> assert e.value() == Decimal("51.51"), e.value()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

during tests e.value() is not visible in error without explicit add as message

* asyncio coroutine was not awaited, so test was false-positive (poor visible on Travis CI)
  Fix test.
* FloatEdit unittests passed, incorrect output doctests not caused exception on CI
* warnings on version number
* warnings about deprecated setup.py usage for tests run
* python2.7 is not able to execute tests for asyncio due to mandatory syntax changes (async def).
  Fix logic:
    1. added `isinstance` check before getting significance
    2. Fix FloatEdit and add corner case tests
    3. normalize version number
    4. execute tests via `unittest`
    5. stop using python 2.7 in tests run instead of increasing complexity
self._decimalSeparator, '.')) * 1
normalized = Decimal(self.edit_text.replace(self._decimalSeparator, '.'))
if self.significance is not None:
return normalized.quantize(self.significance)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://docs.python.org/3/library/decimal.html#decimal.Decimal.quantize should be enough without context magic and manual calculation

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to mention I somehow wasn't using the context anyway...
And made the same mistake in the test case (facepalm).

This is a nice clean alternative.

@penguinolog
Copy link
Collaborator Author

Tests green.
Corner cases now handled correctly, thread context is not modified anymore for Decimal.
Warnings amount reduced (version normalization, not awaited coroutine).

@penguinolog penguinolog merged commit 9e7ec1c into urwid:master Mar 29, 2023
@penguinolog penguinolog deleted the tests_fixes branch March 29, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants