Details
- Reviewers
douardda - Group Reviewers
Reviewers - Commits
- rDGQLce677e0bdeba: Refactor snapshot branch functional tests
Diff Detail
- Repository
- rDGQL GraphQL API
- Branch
- test-refactoring1
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 29879 Build 46706: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 46705: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D7987 (id=28776)
Rebasing onto 67e0bf7370...
Current branch diff-target is up to date.
Changes applied before test
commit 065b17cc5a5f969ffac51df89326e7d15d92bf70
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Jun 14 14:48:28 2022 +0200
Refactor snapshot branch functional testsSee https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/57/ for more details.
please explain a bit what this refactor is about in the commit message.
| swh/graphql/tests/functional/test_branch_connection.py | ||
|---|---|---|
| 11 | some type annotation wouldn't hurt, maybe | |
| 14–22 | 2 comments/questions:
| |
| 63 | not related to this diff, but wouldn't it be better to compare the full result with expected values (rather than just count branches)? | |
| 134 | seeing this params argument make me think the get_branches argument handling could be improved; e.g. make it accept a **kwargs and build the branches() call from these. | |
Build is green
Patch application report for D7987 (id=28797)
Rebasing onto 04b81ece1b...
Current branch diff-target is up to date.
Changes applied before test
commit d978b8b5df7d4b7b465e2fea10f96d5ddd1e3d29
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Jun 14 14:48:28 2022 +0200
Refactor snapshot branch functional tests
Add a get_branches function that can be used by test functions to get
snapshot branches with variable parameters.See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/65/ for more details.
| swh/graphql/tests/functional/test_branch_connection.py | ||
|---|---|---|
| 14–22 | f-strings: T4306 (issue with { in the query body) | |
| 63 | That was missing. Added another test for that (test_get_data) | |
| 134 | This gets difficult for this query. Schema is using Enums to represent target types (revision, release etc). It makes constructing strings from python variables a bit difficult (and it is easy to make hard coded strings like "types: [revision, release]". I will try to fix this as a general function that can be used by all the functional tests. | |
Build is green
Patch application report for D7987 (id=28838)
Rebasing onto ed8d982011...
Current branch diff-target is up to date.
Changes applied before test
commit ce677e0bdebadace7d94788d1f6d93a125e94a53
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date: Tue Jun 14 14:48:28 2022 +0200
Refactor snapshot branch functional tests
Add a get_branches function that can be used by test functions to query
snapshot branches with variable parameters.
Add a utility funtion to build query params string from arguments.
Change origin connection test to use the new utility functionSee https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/67/ for more details.
| swh/graphql/tests/functional/test_branch_connection.py | ||
|---|---|---|
| 14–22 | I added the utility function for building param in this commit itself. | |
| 134 | I added a utility function in this commit itself. (utils.get_query_params_from_args). So the previous comment is not valid anymore. | |
| swh/graphql/tests/functional/utils.py | ||
|---|---|---|
| 34–36 ↗ | (On Diff #28838) | not a big deal, but it's a bit weird to have extracted this one-liner in a dedicated function that's used only once in the code (in get_origins_from_api) and have not unit test (which I'm not convinced it makes really sense to have unitary tested). |
| swh/graphql/tests/functional/utils.py | ||
|---|---|---|
| 34–36 ↗ | (On Diff #28838) | my bad, it's used twice, but still |
| swh/graphql/tests/functional/utils.py | ||
|---|---|---|
| 34–36 ↗ | (On Diff #28838) | In fact, this will be used in almost of the tests to create quries.(already using in test_origin_connection). |
| 34–36 ↗ | (On Diff #28838) | In fact, this will be used in almost all the tests to create queries. I will be adding those tests (for directories, contents etc) using this. |