With some other dev-requirements having no version specification, there is no real reason to keep an upper bound to pytest-sugar either. The lower bound is still relevant as that fixed a bug.
tests: bump pytest version requirement to allow 3.x
pytest is an active project, since our previous requirement setting there have been many releases, with new features and bug fixes.
One of the improvements in pytest-3.0 is that 'yield_fixture' (a fixture that can embed cleanup code after a yield) is no longer different than 'fixture', i.e. a standard fixture supports the yield cleanup mechanism. We will switch to use that when we make 3.0 mandatory.
yield-based tests (tests that are actually generating tests) are a nose feature which were also supported in pytest. However, since pytest-3.0 this type of tests is deprecated, they will be removed in pytest-4.0.
The only yield tests present in the kallithea test suite are simply yielding a method that does a single assert. The benefit of having each of these asserts in a separate test is not very high.
Replace the yield by a direct call to the previously-yielded method. This means the total test count reduces but the actual amount of test code is the same.
test.ini: align some logging-related settings with development.ini
When manually using test.ini (that is, not via the test suite) the coloring of log output like development.ini provides can be very useful. Note that this color output is not taken along by pytest-capturelog, regardless of this patch.
Some extra-verbose logging in test.ini is on the other hand not needed and could be enabled when needed by a specific developer.
Note: the 'level=DEBUG' setting for handler_console_sql is not taken along to test.ini: it causes duplicate sqlalchemy debug logs, one through handler_console_sql and another through another path.
If future experience points out that some of these changes are actually worse than they were, adjustments can still be made.
tests: admin_users: make sure repo group is deleted
test_delete_repo_group_err creates then deletes a repository group. However, if the delete fails the repository group remains. This later causes problems in the model tests.
Introduce a pytest yield fixture to handle the creation _and_ deletion of the repository group (suggested by Søren Løvborg). The creation of the user needs to happen _before_ that of the user group, and we cannot share data between two pytest fixtures, so the user is created in the fixture as well.
tests: admin_users: make sure all custom IP permissions are cleared
test_delete_ip changes IP permissions and at the end tries to clean up by deleting it again. When the delete fails, there is still a restricted IP permission configuration, causing other tests to fail. Use the recently added pytest fixture auto_clear_ip_permissions to fix this. The fixture is extended to not only clear IP permissions for the default user, but also for the 'regular' test user.
Similar cleanup code in test_add_ip is deleted because it serves the same purpose, but would fail to execute if something went wrong earlier in the test method.
This commit is very similar to an earlier commit that covers similar add/delete IP functionality for the default user, in test_permissions.py.
tests: admin_permissions: split test_delete_ips from test_add_ips
While it is not necessary to be pedantic and split each assert in a separate test, it makes sense to create separate tests for separate logical actions. This makes it easy to understand from a test summary what works and what doesn't.
Add and delete are deemed two such separate logical actions. The original test_add_ips test did both add and delete, but was not even named to cover this.
Note that the 'add' in the delete test is not the same as the 'add' in the add test, i.e. for the delete test direct method calls are made instead of passing through self.app.get/post (which is a higher layer of abstraction).
Background of this commit: during the Turbogears2 port, delete actions were not yet functional, and 'test_add_ips' failed as a result even though 'add' was perfectly fine.
tests: admin_permissions: make sure all custom IP permissions are cleared
test_add_ips changes IP permissions and at the end tries to clean up by deleting it again. When the delete fails, there is still a restricted IP permission configuration, causing other tests to fail.
Attempt a direct delete to the database without relying on the application post/get interface.
tests: remove sleep hack to expire sql_cache_short
A number of tests sleep for 1.5 or 2 seconds to let the beaker cache 'sql_cache_short' expire. This cache is for example used for IP permissions; tests changing such permissions need to make sure they take effect before proceeding, especially when exiting from the test.
A much faster method is to effectively invalidating the caches. Because it is difficult and fragile to only invalidate the relevant cache -- difficult to know exactly which cache needs to be invalidated, fragile because the string indicating the cache line is not very nice and might change in the future (in the case of IP permissions for the default user, the cache is referred to with something like "get_user_ips_default_Mapper|UserIpMap|users". This string changes when the permissions are for a different user.
Clearing all caches shouldn't be a problem in a test context.
Running pip install -e . in a clean environment (or --upgrade), would cause: "ContextualVersionConflict: (WebOb 1.1.1 (.../lib/python2.7/site-packages), Requirement.parse('WebOb>=1.2'), set(['WebTest']))"
setup.py required webob>=1.0.8,<=1.1.1 which pip resolved as WebOb==1.1.1. setup.py required Pylons which requires WebTest>=1.3.1. Pip would thus install the latest WebTest-2.0.23 ... which however has a WebOb>=1.2 dependency. Pip only makes one pass at resolving dependencies and it would not resolve or notice this inconsistency.
This problem came with dd676fdeda0f which moved pinning of WebTest 1.4.3 out of setup.py.
Since Pylons require webtest anyway, there is no point in having it as a dev requirement only - put it back in setup.py.
This might just be a temporary workaround until the WebOb version is updated.
Original patch by Dominik has been rewritten by Mads.
* Mention "The Thing" in the header, link to online version * Show The Thing * A dense summary of the essentials of the context below it
The html indentation is odd in order to make the next diff smaller.
The text version is based on:
for a in kallithea/templates/email_templates/*.html; do sed -e 's,<\([^%/>]\|/[^%>]\)*>,,g' -e 's,\.html",\.txt",g' -e 's,^ *,,g' -e 's/}/|n,unicode}/g' $a > ${a%%.html}.txt ; done
branches: fix performance of branch selectors with many branches - only show the first 200 results
The way we use select2, it will cause browser performance problems when a select list contains thousands of entries. The primary bottleneck is the DOM creation, secondarily for the query to filter through the entries and decide what to show. We thus primarily have to limit how many entries we put in the drop-down, secondarily limit the iteration over data.
One tricky case is where the user specifies a short but full branch name (like 'trunk') but many other branches contains the same string (not necessarily at the beginning, like 'for-trunk-next-week') which come before the perfect match in the branch list. It is thus not a solution to just stop searching when a fixed amount of matches have been found.
Instead, we limit the amount of ordinary query matches, but always show all prefix matches. We thus always have to iterate through all entries, but we start using the (presumably) cheaper prefix search when the limit has been reached.
There is no filtering initially when there is no query term, so that case has to be handled specially.
Upstream select2 is now at 4.x. Upgrading is not trivial, and getting this fixed properly upstream is not a short term solution. Instead, we customize our copy. The benefit from this patch is bigger than the overhead of "maintaining" it locally.
files: always show the requested version - not last changeset that touched the file
The last changeset to touch the file is rarely relevant - the whole annotate of individual lines of code will give that kind of information in a much more relevant way. The information is however available in the history drophown.
This also gets rid of some weird revision compare.
hg: drop pointless push_ssl configuration setting - if there is a risk push can be compromised, credentials can also easily be stolen for pull
Everybody should have a ssl-only setup now. Alternatively, there is a use case for 'only anonymous traffic on ssl - all authentication and authenticated traffic must be on ssl'. That can be done with proper web server configuration.
_get_instance is a BaseModel method, but never uses "self". Instead it takes a class argument, which indicates that it's better suited as a classmethod on said classes.
Also rename to something more descriptive, remove leading underscore since it's not a private API, and refactor for readability.
__get_lem and __get_index_filenames has nothing in common with the rest of util2, neither with respects to implementation nor usage, and belongs in a dedicated module. Also, they are clearly not actually private, so shouldn't be named with leading underscores.
compare: ensure that repositories exist before proceeding
The index method on CompareController did not verify that other_repo existed, causing a rendering error if it wasn't.
Since neither controller method can proceed if either repository is non-existent, check existence and load Repository objects in __before__. Also perform type compatibility check up front while we're at it, remove redundant repository database lookups, and enable error message i18n.
paster: split paster specifics out of kallithea.lib.utils
BasePasterCommand and ask_ok are only useful in a Paster/command-line context, and can thus be removed from the already overly cluttered main utils module.
(The new common.py has been added to Mercurial as a copy of utils.py, preserving its file history, but creating a somewhat bewildering diff.)
First, find all calls to HasPermissionAll with only a single permission given, and convert to equivalent calls to HasPermissionAny.
Next, observe that it's hard to envision situations requiring multiple permissions (of the same scope: global/repo/repo group) to be satisfied. Sufficiently hard that there are actually no such examples in the code.
Finally, considering that (should it ever be needed) HasPermissionAll can be trivially built as a conjunction of HasPermissionAny calls (the decorators, too) with only a small performance impact, simply remove HasPermissionAll and related classes and functions.
A log level of WARNING meant that Alembic was entirely silent when nothing went wrong. With INFO, Alembic actually shows what migrations it is running.
kallithea/tests/models/test_dump_html_mails.ref.html is expected to contain trailing whitespace in the "-- " de-facto standard signature separator (as e.g. referenced in RFC 3676, section 4.3).
The subject line is used for mail threading in gmail and can thus not be changed without impacting users ... but now we do it.
* The tag '[Review]' is more spot-on than '[Added]'. * The subject should be short so it fits on one line, so abbreviate "pull request" to PR. * Add the PR owner - convenient for filtering comments on own PRs from comments on other PRs.