Page MenuHomeSoftware Heritage
Differential D1720

Modify API output and test
AbandonedPublic
Actions

Authored by vlorentz on Jul 10 2019, 5:31 PM.

Details

Reviewers
twitu
Group Reviewers
Reviewers
Summary

In reference to T1433

Changed return type for content_fossology_license_get.
It now returns dict with sha1 key and list of license
and tool information as value.

Diff Detail

Repository
rDCIDX Metadata indexer
Branch
refactor_license_api
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7221
Build 10198: tox-on-jenkinsJenkins
Build 10197: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
swh-public-ci added a comment.Jul 10 2019, 5:32 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/568/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/568/console

Harbormaster failed remote builds in B6794: Diff 5774!Jul 10 2019, 5:32 PM
twitu updated this revision to Diff 5775.Jul 10 2019, 5:36 PM

small edit and rebase

swh-public-ci added a comment.Jul 10 2019, 5:37 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/571/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/571/console

Harbormaster failed remote builds in B6795: Diff 5775!Jul 10 2019, 5:37 PM
twitu retitled this revision from Modify output and test to Modify API output and test.Jul 10 2019, 5:39 PM
vlorentz requested changes to this revision.Jul 11 2019, 3:16 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/indexer/storage/__init__.py
445

Use @db_transaction() instead

swh/indexer/storage/in_memory.py
438–442

Good use of defaultdict, thanks

This revision now requires changes to proceed.Jul 11 2019, 3:16 PM
twitu updated this revision to Diff 5807.Jul 11 2019, 8:12 PM

Use db_transaction annotation

swh-public-ci added a comment.Jul 11 2019, 8:13 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/572/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/572/console

Harbormaster failed remote builds in B6847: Diff 5807!Jul 11 2019, 8:13 PM
twitu updated this revision to Diff 5808.Jul 11 2019, 8:24 PM
  • Modify output and test
  • In reference to T1433
  • In reference to T1433
  • Use db_transaction annotation
swh-public-ci added a comment.Jul 11 2019, 8:25 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/574/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/574/console

Harbormaster failed remote builds in B6848: Diff 5808!Jul 11 2019, 8:25 PM
vlorentz requested changes to this revision.Jul 18 2019, 11:20 AM
This revision now requires changes to proceed.Jul 18 2019, 11:20 AM
twitu updated this revision to Diff 5919.Jul 21 2019, 10:44 PM
  • Fix test
swh-public-ci added a comment.Jul 21 2019, 10:46 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/582/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/582/console

Harbormaster failed remote builds in B6955: Diff 5919!Jul 21 2019, 10:46 PM
twitu updated this revision to Diff 5921.Jul 22 2019, 3:53 PM
  • Change expected result
swh-public-ci added a comment.Jul 22 2019, 3:56 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/583/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/583/console

Harbormaster failed remote builds in B6957: Diff 5921!Jul 22 2019, 3:56 PM
vlorentz requested changes to this revision.Aug 5 2019, 1:49 PM

Do you want help with your diff?

This revision now requires changes to proceed.Aug 5 2019, 1:49 PM
twitu added a comment.Aug 6 2019, 4:30 AM

I haven't been able to give enough time to it. I'll complete the diff by this weekend.

twitu updated this revision to Diff 6193.Aug 11 2019, 4:26 PM
  • Change tests and expected results
Harbormaster failed remote builds in B7221: Diff 6193!Aug 11 2019, 4:28 PM
swh-public-ci added a comment.Aug 11 2019, 4:28 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/592/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/592/console

twitu added a comment.Aug 12 2019, 5:50 AM

It appears that two different tests TestFossologyLicenseRangeIndexer and TestMimetypeRangeIndexer are inheriting from the same class namely, CommonContentIndexerRangeTest as a result both use the same assert_results_ok method. Both test classes have the same structure for expected results, hence the test works for both, as I experimented in the previous commit, changing the test for the new format of TestFossologyLicenseRangeIndexer will fail for TestMimetypeRangeIndexer. How do I resolve this? Should I override the method in both classes?

The case for CommonContentIndexerTest is similar as it is inherited by TestCtagsIndexer, TestFossologyIndexer and TestMimeTypeIndexer.

vlorentz added a comment.Aug 12 2019, 11:34 AM
In D1720#42747, @twitu wrote:

How do I resolve this? Should I override the method in both classes?

Only override it in TestFossologyLicenseRangeIndexer.

twitu updated this revision to Diff 6205.Aug 12 2019, 5:22 PM
  • Override test methods
swh-public-ci added a comment.Aug 12 2019, 5:24 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/595/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/595/console

Harbormaster failed remote builds in B7236: Diff 6205!Aug 12 2019, 5:24 PM
twitu updated this revision to Diff 6206.Aug 12 2019, 5:28 PM
  • Override test methods
swh-public-ci added a comment.Aug 12 2019, 5:31 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/596/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/596/console

Harbormaster failed remote builds in B7237: Diff 6206!Aug 12 2019, 5:31 PM
twitu updated this revision to Diff 6218.Aug 13 2019, 9:59 PM
  • Fix return value for license indexer
swh-public-ci added a comment.Aug 13 2019, 10:01 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/597/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/597/console

Harbormaster failed remote builds in B7250: Diff 6218!Aug 13 2019, 10:01 PM
twitu updated this revision to Diff 6219.Aug 13 2019, 10:24 PM
  • Fix return value for license indexer
Harbormaster failed remote builds in B7251: Diff 6219!Aug 13 2019, 10:26 PM
swh-public-ci added a comment.Aug 13 2019, 10:26 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/598/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/598/console

twitu added a comment.EditedAug 13 2019, 10:42 PM

Overiding just the test method doesn't work for TestFossologyLicenseRangeIndexer. Tests test__index_contents and test__index_contents_with_indexed_data call the function self.indexer._index_contents which is implemented in ContentRangeIndexer.

It calls a function index which is not implemented and it yields value when the docstring says it should return a dictionary. I am not sure how to resolve this.

twitu updated this revision to Diff 6220.Aug 14 2019, 5:27 AM
  • Fix return value for license indexer
swh-public-ci added a comment.Aug 14 2019, 5:30 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCIDX/job/tox/599/ for more details.

Harbormaster completed remote builds in B7252: Diff 6220.Aug 14 2019, 5:30 AM
twitu added a comment.Aug 14 2019, 5:36 AM
In D1720#42908, @twitu wrote:

Overiding just the test method doesn't work for TestFossologyLicenseRangeIndexer. Tests test__index_contents and test__index_contents_with_indexed_data call the function self.indexer._index_contents which is implemented in ContentRangeIndexer.

It calls a function index which is not implemented and it yields value when the docstring says it should return a dictionary. I am not sure how to resolve this.

Keeping the expected results unchanged allows the build to pass, even though it is in the older format. However, as I have quoted, I am not sure how the actual results for the range is being generated by _index_contents. Should I write a new method that intercepts the return value of _index_contents and converts it to the required format? However is such a change within the ambit of this diff?

vlorentz added a comment.Aug 14 2019, 10:23 AM

Keeping the expected results unchanged allows the build to pass, even though it is in the older format.

Is it?

swh/indexer/tests/test_fossology_license.py
123–125

This conditional is not needed

127–128

Neither is this one

131–132

I'm not sure this one is either

134–136

You can replace this for loop with simply: self.assertEqual(expected_results, actual_results).
This will also check there is no extra/unwanted items in actual_results.

vlorentz requested changes to this revision.Aug 19 2019, 11:32 AM
This revision now requires changes to proceed.Aug 19 2019, 11:32 AM
twitu updated this revision to Diff 6281.Aug 20 2019, 6:11 AM
  • Remove unneeded checks
swh-public-ci added a comment.Aug 20 2019, 6:15 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/600/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/600/console

Harbormaster failed remote builds in B7317: Diff 6281!Aug 20 2019, 6:15 AM
twitu updated this revision to Diff 6282.Aug 20 2019, 6:18 AM
twitu marked 4 inline comments as done.
  • Remove unneeded checks
swh-public-ci added a comment.Aug 20 2019, 6:21 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/601/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/601/console

Harbormaster failed remote builds in B7318: Diff 6282!Aug 20 2019, 6:21 AM
twitu updated this revision to Diff 6283.Aug 20 2019, 6:33 AM
  • Remove unneeded checks
swh-public-ci added a comment.Aug 20 2019, 6:35 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/602/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/602/console

Harbormaster failed remote builds in B7319: Diff 6283!Aug 20 2019, 6:35 AM
twitu updated this revision to Diff 6284.Aug 20 2019, 6:49 AM
  • Remove unneeded checks
swh-public-ci added a comment.Aug 20 2019, 6:51 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/603/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/603/console

Harbormaster failed remote builds in B7320: Diff 6284!Aug 20 2019, 6:51 AM
vlorentz requested changes to this revision.Sep 19 2019, 5:30 PM

@twitu Hi, any news on this?

This revision now requires changes to proceed.Sep 19 2019, 5:30 PM
twitu added a comment.Sep 22 2019, 1:00 PM

I am sorry for the delay. Due to some dependency issue, I am unable to run the tests locally which is why I have been making so many pushes and am unable to solve it effectively. I will reinstall all the whole environment and try to fix it, if the environment works I will complete this. If I am unable to fix my environment then sadly I will have to drop this one.

vlorentz added a comment.Sep 22 2019, 1:36 PM

Did you try tox --recreate?

twitu added a comment.Sep 22 2019, 1:40 PM

The problem is with the python setup something to do with typing.py giving errors if I remember correctly. I will try it again and add a error log.

vlorentz added a comment.Apr 29 2021, 2:16 PM

We refactored this code, so I'm going to close this diff

vlorentz commandeered this revision.Apr 29 2021, 2:17 PM
vlorentz abandoned this revision.
vlorentz edited reviewers, added: twitu; removed: vlorentz.

Revision Contents
Changeset List

PathSize
swh/
indexer/
storage/
9 lines
7 lines
tests/
storage/
12 lines
27 lines
10 lines

Diff 6193

View Options

swh/indexer/storage/__init__.py

Loading...
View Options

swh/indexer/storage/in_memory.py

Loading...
View Options

swh/indexer/tests/storage/test_storage.py

Loading...
View Options

swh/indexer/tests/test_fossology_license.py

Loading...
View Options

swh/indexer/tests/utils.py

Loading...
About · Developer information · Legal