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
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6847
Build 9582: tox-on-jenkinsJenkins
Build 9581: 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–441 ↗(On Diff #5775)

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
125–127 ↗(On Diff #6220)

This conditional is not needed

129–130 ↗(On Diff #6220)

Neither is this one

133–134 ↗(On Diff #6220)

I'm not sure this one is either

136–138 ↗(On Diff #6220)

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/
2 lines

Diff 5807

View Options

swh/indexer/storage/__init__.py

Loading...
About · Developer information · Legal