Page MenuHomeSoftware Heritage
Differential D1420 Diff 4784 swh/web/tests/common/test_service.py

Changeset View

Standalone View

View Options

swh/web/tests/common/test_service.py

# Copyright (C) 2015-2019 The Software Heritage developers # Copyright (C) 2015-2019 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU Affero General Public License version 3, or any later version # License: GNU Affero General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
import itertools import itertools
import pytest import pytest
import random import random
from collections import defaultdict from collections import defaultdict
from hypothesis import given from hypothesis import given
from swh.model.hashutil import hash_to_bytes, hash_to_hex from swh.model.hashutil import hash_to_bytes, hash_to_hex
anlambertUnsubmitted
Done
Inline Actions

As you are only using the DentryPerms enum from that module, you should change your import statement to:

from swh.model.from_disk import DentryPerms
anlambert: As you are only using the `DentryPerms` enum from that module, you should change your import…
from swh.web.common import service from swh.web.common import service
from swh.web.common.exc import BadInputExc, NotFoundExc from swh.web.common.exc import BadInputExc, NotFoundExc
from swh.web.tests.strategies import ( from swh.web.tests.strategies import (
content, contents, unknown_content, unknown_contents, content, contents, unknown_content, unknown_contents,
contents_with_ctags, origin, new_origin, visit_dates, directory, contents_with_ctags, origin, new_origin, visit_dates, directory,
release, revision, unknown_revision, revisions, unknown_revisions, release, revision, unknown_revision, revisions, unknown_revisions,
ancestor_revisions, non_ancestor_revisions, invalid_sha1, sha256, ancestor_revisions, non_ancestor_revisions, invalid_sha1, sha256,
▲ Show 20 Lines • Show All 269 Lines • ▼ Show 20 Lines def test_lookup_revision_with_context_ko_sha1_git_does_not_exist(
service.lookup_revision_with_context(sha1_git_root, sha1_git) service.lookup_revision_with_context(sha1_git_root, sha1_git)
self.assertIn('Revision %s not found' % sha1_git, cm.exception.args[0]) self.assertIn('Revision %s not found' % sha1_git, cm.exception.args[0])
@given(revision(), unknown_revision()) @given(revision(), unknown_revision())
def test_lookup_revision_with_context_ko_root_sha1_git_does_not_exist( def test_lookup_revision_with_context_ko_root_sha1_git_does_not_exist(
self, revision, unknown_revision): self, revision, unknown_revision):
sha1_git_root = unknown_revision sha1_git_root = unknown_revision
sha1_git = revision sha1_git = revision
with self.assertRaises(NotFoundExc) as cm: with self.assertRaises(NotFoundExc) as cm:
service.lookup_revision_with_context(sha1_git_root, sha1_git) service.lookup_revision_with_context(sha1_git_root, sha1_git)
self.assertIn('Revision root %s not found' % sha1_git_root, self.assertIn('Revision root %s not found' % sha1_git_root,
cm.exception.args[0]) cm.exception.args[0])
@given(ancestor_revisions()) @given(ancestor_revisions())
def test_lookup_revision_with_context(self, ancestor_revisions): def test_lookup_revision_with_context(self, ancestor_revisions):
sha1_git = ancestor_revisions['sha1_git'] sha1_git = ancestor_revisions['sha1_git']
Show All 28 Linesclass ServiceTestCase(WebTestCase):
@given(unknown_revision()) @given(unknown_revision())
def test_lookup_directory_with_revision_not_found(self, unknown_revision): def test_lookup_directory_with_revision_not_found(self, unknown_revision):
with self.assertRaises(NotFoundExc) as cm: with self.assertRaises(NotFoundExc) as cm:
service.lookup_directory_with_revision(unknown_revision) service.lookup_directory_with_revision(unknown_revision)
self.assertIn('Revision %s not found' % unknown_revision, self.assertIn('Revision %s not found' % unknown_revision,
cm.exception.args[0]) cm.exception.args[0])
@given(revision()) @given(revision(), unknown_content())
def test_lookup_directory_with_revision_ko_path_to_nowhere(self, revision): def test_lookup_directory_with_revision_unknown_content(
self, revision, unknown_content):
dir_path = "README.md"
sha1_git_bin = service._to_sha1_bin(revision)
revision_ = service._first_element(
self.storage.revision_get([sha1_git_bin])
vlorentzUnsubmitted
Done
Inline Actions

You can remove that line

vlorentz: You can remove that line
anlambertUnsubmitted
Done
Inline Actions

Yes, please remove it

anlambert: Yes, please remove it
)
revision_['directory'] = unknown_content['sha1_git']
revision_['type'] = 'file'
revision_['id'] = revision[0: len(revision)-2]+"00"
revision_['target'] = unknown_content['sha1_git']
self.storage.revision_add([revision_])
with self.assertRaises(NotFoundExc) as cm:
service.lookup_directory_with_revision(revision, dir_path)
self.assertIn('Content not found for %s' % revision,
cm.exception.args[0])
vlorentzUnsubmitted
Done
Inline Actions

please use unknown_revision() instead of revision(), that will spare some hacks in the test

You can also use unknown_directory() to generate a directory id, and build the directory from scratch in the test (that's easier to read)

vlorentz: please use `unknown_revision()` instead of `revision()`, that will spare some hacks in the test…
@given(revision())
def test_lookup_directory_with_revision_ko_path_to_nowhere(
self, revision):
invalid_path = 'path/to/something/unknown' invalid_path = 'path/to/something/unknown'
with self.assertRaises(NotFoundExc) as cm: with self.assertRaises(NotFoundExc) as cm:
service.lookup_directory_with_revision(revision, invalid_path) service.lookup_directory_with_revision(revision, invalid_path)
exception_text = cm.exception.args[0].lower() exception_text = cm.exception.args[0].lower()
self.assertIn('directory or file', exception_text) self.assertIn('directory or file', exception_text)
self.assertIn(invalid_path, exception_text) self.assertIn(invalid_path, exception_text)
self.assertIn('revision %s' % revision, exception_text) self.assertIn('revision %s' % revision, exception_text)
self.assertIn('not found', exception_text) self.assertIn('not found', exception_text)
@given(revision_with_submodules()) @given(revision_with_submodules())
def test_lookup_directory_with_revision_submodules( def test_lookup_directory_with_revision_submodules(
self, revision_with_submodules): self, revision_with_submodules):
anlambertUnsubmitted
Done
Inline Actions

You should use the hash_to_bytes function from swh.model.hashutil instead

anlambert: You should use the `hash_to_bytes` function from `swh.model.hashutil` instead
rev_sha1_git = revision_with_submodules['rev_sha1_git'] rev_sha1_git = revision_with_submodules['rev_sha1_git']
rev_dir_path = revision_with_submodules['rev_dir_rev_path'] rev_dir_path = revision_with_submodules['rev_dir_rev_path']
actual_data = service.lookup_directory_with_revision( actual_data = service.lookup_directory_with_revision(
vlorentzUnsubmitted
Done
Inline Actions

Code style nitpick:

revision = {
    'author': {
        'name': b'abcd',
        ...
vlorentz: Code style nitpick: ``` revision = { 'author': { 'name': b'abcd', ... ```
rev_sha1_git, rev_dir_path) rev_sha1_git, rev_dir_path)
anlambertUnsubmitted
Done
Inline Actions

Again some nitpicks regarding code formatting, please update that dict definition to:

revision = {
    'author': {
        'name': b'abcd',
        'email': b'abcd@company.org',
        'fullname': b'abcd abcd'
    },
    'committer': {
        'email': b'aaaa@company.org',
        'fullname': b'aaaa aaa',
        'name': b'aaa'
    },
    'committer_date': {
        'negative_utc': False,
        'offset': 0,
        'timestamp': 1437511651
    },
    'date': {
        'negative_utc': False,
        'offset': 0,
        'timestamp': 1437511651
    },
    'message': b'bleh',
    'metadata': [],
    'parents': [],
    'synthetic': False,
    'type': 'file',
    'id': hash_to_bytes(unknown_revision),
    'directory': hash_to_bytes(unknown_directory)
}
anlambert: Again some nitpicks regarding code formatting, please update that dict definition to…
revision = self.revision_get(revision_with_submodules['rev_sha1_git']) revision = self.revision_get(revision_with_submodules['rev_sha1_git'])
directory = self.directory_ls(revision['directory']) directory = self.directory_ls(revision['directory'])
vlorentzUnsubmitted
Done
Inline Actions

Can you replace unknown_revision[:len(unknown_revision)-4]+"bcd1" with just unknown_revision?

vlorentz: Can you replace `unknown_revision[:len(unknown_revision)-4]+"bcd1"` with just…
rev_entry = next(e for e in directory if e['name'] == rev_dir_path) rev_entry = next(e for e in directory if e['name'] == rev_dir_path)
anlambertUnsubmitted
Done
Inline Actions

unindent here

anlambert: unindent here
expected_data = { expected_data = {
anlambertUnsubmitted
Done
Inline Actions

same here

anlambert: same here
'content': self.revision_get(rev_entry['target']), 'content': self.revision_get(rev_entry['target']),
'path': rev_dir_path, 'path': rev_dir_path,
'revision': rev_sha1_git, 'revision': rev_sha1_git,
'type': 'rev' 'type': 'rev'
} }
self.assertEqual(actual_data, expected_data) self.assertEqual(actual_data, expected_data)
@given(revision()) @given(revision())
anlambertUnsubmitted
Done
Inline Actions

Same here, update the dict definition to:

dir = {
    'id': hash_to_bytes(unknown_directory),
    'entries': [{
        'name': bytes(dir_path.encode('utf-8')),
        'type': 'file',
        'target': hash_to_bytes(unknown_content['sha1_git']),
        'perms': DentryPerms.content
    }]
}
anlambert: Same here, update the dict definition to: ```lang=python dir = { 'id': hash_to_bytes…
def test_lookup_directory_with_revision_without_path(self, revision): def test_lookup_directory_with_revision_without_path(self, revision):
actual_directory_entries = \ actual_directory_entries = \
service.lookup_directory_with_revision(revision) service.lookup_directory_with_revision(revision)
revision_data = self.revision_get(revision) revision_data = self.revision_get(revision)
expected_directory_entries = \ expected_directory_entries = \
self.directory_ls(revision_data['directory']) self.directory_ls(revision_data['directory'])
self.assertEqual(actual_directory_entries['type'], 'dir') self.assertEqual(actual_directory_entries['type'], 'dir')
anlambertUnsubmitted
Done
Inline Actions

Same code formatting issue, replace with:

with self.assertRaises(NotFoundExc) as cm:
    service.lookup_directory_with_revision(unknown_revision, dir_path)
self.assertIn('Content not found for revision %s' % unknown_revision,
              cm.exception.args[0])
anlambert: Same code formatting issue, replace with: ```lang=python with self.assertRaises(NotFoundExc) as…
self.assertEqual(actual_directory_entries['content'], self.assertEqual(actual_directory_entries['content'],
expected_directory_entries) expected_directory_entries)
@given(revision()) @given(revision())
def test_lookup_directory_with_revision_with_path(self, revision): def test_lookup_directory_with_revision_with_path(self, revision):
revision_data = self.revision_get(revision) revision_data = self.revision_get(revision)
dir_entries = [e for e in self.directory_ls(revision_data['directory']) dir_entries = [e for e in self.directory_ls(revision_data['directory'])
▲ Show 20 Lines • Show All 424 Lines • Show Last 20 Lines
About · Developer information · Legal