Details

Reviewers

zack
seirl

Maniphest Tasks

T1920: graph service: add tests for the python client

Commits

rDGRPHc57819ce34ae: client: add API endpoints integration tests
rDGRPH293eb8a57c9f: server: add --port/-p flag option
rDGRPHa098482cba3e: server: switch version numbering from 1.0 to 0.0.1

Summary

Closes T1920.

(python:venv) ↪ pytest
========================================================================== test session starts ===========================================================================
platform linux -- Python 3.5.3, pytest-3.10.1, py-1.8.0, pluggy-0.12.0
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/home/haltode/work/swh/swh-environment/swh-graph/.hypothesis/examples')
rootdir: /home/haltode/work/swh/swh-environment/swh-graph, inifile: pytest.ini
plugins: postgresql-1.4.1, hypothesis-4.32.2, celery-4.3.0, kafka-0.3.1, django-3.5.1, requests-mock-1.6.0
collected 6 items

swh/graph/tests/test_api_client.py ......                               [100%]

======================================================================= 6 passed in 11.69 seconds ========================================================================

Diff Detail

Repository

rDGRPH Compressed graph representation

Branch

client-integration-tests

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 7108
Build 10021: arc lint + arc unit

Event Timeline

haltode created this revision.Jul 31 2019, 2:01 PM

Harbormaster completed remote builds in B7080: Diff 6040.Jul 31 2019, 2:01 PM

seirl requested changes to this revision.Jul 31 2019, 2:37 PM

seirl added inline comments.

swh/graph/tests/test_api_client.py
12	You never seem to use the "server" variable elsewhere in the tests (with good reasons, everything should go through the client, not the process). Maybe you should just yield "client", and rename this whole fixture "graph_client"?
13	Put an root to the path here, even if it's a reference to `os.path.dirname(__file__)`. You don't want to depend on CWD.
13	Also, it looks like you would have an easier time using pathlib here. python from pathlib import Path java_dir = Path(__file__).parent / 'java/server' pom_path = java_dir / 'pom.xml' jar_path = java_dir / 'target/swh-graph-...'
22	I guess this is not going to stay "1.0" forever. Is there a way we could make that generic?
34	Is it possible to give a port parameter to the server? This looks like a recipe for flakiness. Maybe find a random unused port instead, like this? https://docs.aiohttp.org/en/stable/testing.html#aiohttp.test_utils.aiohttp_unused_port
37	Just yield "server"
43	Is there a particular reason to use unittest instead of regular pytest test cases?
46	Just self.client = client
73	You should probably also verify the types of the results here.

This revision now requires changes to proceed.Jul 31 2019, 2:37 PM

seirl added inline comments.Jul 31 2019, 2:39 PM

swh/graph/tests/test_api_client.py
97	Why are we using CountEqual here? Wouldn't it be better to check that the results are exact?

seirl added inline comments.Jul 31 2019, 2:49 PM

swh/graph/tests/test_api_client.py
73	So yeah, check the types of the results that are completely unpredictable, and check the values that you know will stay constant (#edges, #nodes, etc)

haltode added inline comments.Jul 31 2019, 4:20 PM

swh/graph/tests/test_api_client.py
22	I can put it into `__init__` as a constant, but the swh-graph version number should be even more global than tests. I need to look at others swh-* repos to see what is commonly used.
34	Right now no, but this can be added easily on the Java part as a CLI optional argument.

Do not yield unused server internal variable
Use pathlib instead of os.path
Remove unittest test case
Stronger assertions on types and values in test_stats

Harbormaster completed remote builds in B7093: Diff 6050.Jul 31 2019, 4:34 PM

haltode marked 9 inline comments as done.Jul 31 2019, 4:35 PM

seirl added inline comments.Jul 31 2019, 4:39 PM

swh/graph/tests/test_api_client.py
86	Use `isinstance` instead

seirl requested changes to this revision.Jul 31 2019, 4:41 PM

seirl added inline comments.

swh/graph/tests/test_api_client.py
12	Just so that people aren't confused by the parents[3], maybe split into: swh_graph_root = Path(__file__).parents[3] java_dir = swh_graph_root / 'java/server' ?

This revision now requires changes to proceed.Jul 31 2019, 4:41 PM

Use isinstance instead of type
Simplify pathlib paths
Use sets to compare stats json keys

Harbormaster completed remote builds in B7095: Diff 6052.Jul 31 2019, 5:04 PM

haltode marked 2 inline comments as done.Jul 31 2019, 5:04 PM

Use any available port for server/client interaction
Add version constant in tests

Harbormaster completed remote builds in B7104: Diff 6061.Aug 1 2019, 11:36 AM

haltode marked 2 inline comments as done.Aug 1 2019, 11:36 AM

seirl accepted this revision.Aug 1 2019, 2:13 PM

seirl added inline comments.

swh/graph/tests/test_api_client.py
23	Nit: put that in a function maybe? So that it's easier to replace if we put that in swh.core or something.

This revision is now accepted and ready to land.Aug 1 2019, 2:13 PM

Use aiohttp.test_utils.unused_port()

Harbormaster completed remote builds in B7106: Diff 6069.Aug 1 2019, 2:34 PM

Rebasing on master.

Harbormaster completed remote builds in B7108: Diff 6071.Aug 1 2019, 2:48 PM

Closed by commit rDGRPH293eb8a57c9f: server: add --port/-p flag option (authored by haltode). · Explain WhyAug 1 2019, 2:49 PM

This revision was automatically updated to reflect the committed changes.

Client: add API endpoints integration tests
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 6071

docs/docker.rst

java/server/README.md

java/server/pom.xml

java/server/src/main/java/org/softwareheritage/graph/AllowedEdges.java

java/server/src/main/java/org/softwareheritage/graph/App.java

java/server/src/main/java/org/softwareheritage/graph/Endpoint.java

java/server/src/main/java/org/softwareheritage/graph/Graph.java

java/server/src/main/java/org/softwareheritage/graph/Neighbors.java

java/server/src/main/java/org/softwareheritage/graph/Node.java

java/server/src/main/java/org/softwareheritage/graph/SwhId.java

java/server/src/main/java/org/softwareheritage/graph/SwhPath.java

java/server/src/main/java/org/softwareheritage/graph/algo/Stats.java

java/server/src/main/java/org/softwareheritage/graph/algo/Traversal.java

java/server/src/main/java/org/softwareheritage/graph/backend/MapFile.java

java/server/src/main/java/org/softwareheritage/graph/backend/NodeIdMap.java

java/server/src/main/java/org/softwareheritage/graph/backend/NodeTypesMap.java

java/server/src/main/java/org/softwareheritage/graph/backend/Setup.java

java/server/src/main/java/org/softwareheritage/graph/benchmark/LinuxLog.java

swh/graph/tests/init.py

swh/graph/tests/test_api_client.py

Client: add API endpoints integration testsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 6071

docs/docker.rst

java/server/README.md

java/server/pom.xml

java/server/src/main/java/org/softwareheritage/graph/AllowedEdges.java

java/server/src/main/java/org/softwareheritage/graph/App.java

java/server/src/main/java/org/softwareheritage/graph/Endpoint.java

java/server/src/main/java/org/softwareheritage/graph/Graph.java

java/server/src/main/java/org/softwareheritage/graph/Neighbors.java

java/server/src/main/java/org/softwareheritage/graph/Node.java

java/server/src/main/java/org/softwareheritage/graph/SwhId.java

java/server/src/main/java/org/softwareheritage/graph/SwhPath.java

java/server/src/main/java/org/softwareheritage/graph/algo/Stats.java

java/server/src/main/java/org/softwareheritage/graph/algo/Traversal.java

java/server/src/main/java/org/softwareheritage/graph/backend/MapFile.java

java/server/src/main/java/org/softwareheritage/graph/backend/NodeIdMap.java

java/server/src/main/java/org/softwareheritage/graph/backend/NodeTypesMap.java

java/server/src/main/java/org/softwareheritage/graph/backend/Setup.java

java/server/src/main/java/org/softwareheritage/graph/benchmark/LinuxLog.java

swh/graph/tests/__init__.py

swh/graph/tests/test_api_client.py

Client: add API endpoints integration tests
ClosedPublic
Actions

Revision Contents
Changeset List

swh/graph/tests/init.py