Page MenuHomeSoftware Heritage

(fix ci) Fix search journal client tests
ClosedPublic

Authored by ardumont on May 13 2020, 2:42 PM.

Details

Summary

This:

  • tests: drop the now broken MockedKafkaConsumer test class
  • migrate the journal client test to pytest (it would not accept to inject fixtures otherwise)
  • fixes the ci [1]

[1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

Related to D3144

Test Plan

tox

Diff Detail

Repository
rDSEA Archive search
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 12365
Build 18755: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 18754: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D3151 (id=11181)

Rebasing onto 4dbcbc4e7d...

Current branch diff-target is up to date.
Changes applied before test
commit 603fb64cd7b548243220fd3938fd28a204385465
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 13 13:48:01 2020 +0200

    Fix search journal client tests
    
    This:
    - tests: drop the now broken MockedKafkaConsumer test class.
    - migrate the journal client test to pytest
    - fixes the ci [1]
    
    [1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

Link to build: https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/7/
See console output for more information: https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/7/console

Keep initial assertions order

Build has FAILED

Patch application report for D3151 (id=11182)

Rebasing onto 4dbcbc4e7d...

Current branch diff-target is up to date.
Changes applied before test
commit 68ea36fdecba37375fdabf0aa1c5d12414022b08
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 13 13:48:01 2020 +0200

    Fix search journal client tests
    
    This:
    - tests: drop the now broken MockedKafkaConsumer test class.
    - migrate the journal client test to pytest
    - fixes the ci [1]
    
    [1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

Link to build: https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/8/
See console output for more information: https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/8/console

Fix tests (in the end, reset issue)

Build is green

Patch application report for D3151 (id=11183)

Rebasing onto 4dbcbc4e7d...

Current branch diff-target is up to date.
Changes applied before test
commit 89b47c37d07540e587336c1a820cb129420fdbd6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 13 13:48:01 2020 +0200

    Fix search journal client tests
    
    This:
    - tests: drop the now broken MockedKafkaConsumer test class.
    - migrate the journal client test to pytest
    - fixes the ci [1]
    
    [1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

See https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/9/ for more details.

ardumont edited the summary of this revision. (Show Details)
  • Drop no longer required setUp method
  • Specify the deinitialize/initialize in comments

Build is green

Patch application report for D3151 (id=11184)

Rebasing onto 4dbcbc4e7d...

Current branch diff-target is up to date.
Changes applied before test
commit 5589f9d0e04938008d5acbeb9231c0e91acd335b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 13 13:48:01 2020 +0200

    Fix search journal client tests
    
    This:
    - tests: drop the now broken MockedKafkaConsumer test class.
    - migrate the journal client test to pytest
    - fixes the ci [1]
    
    [1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

See https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/10/ for more details.

swh/search/tests/test_cli.py
37

why this hardcoded IP for the broker?

swh/search/tests/test_cli.py
37

Maybe because it was mock before. This one is really used by the last tests in
this test file, the ones with failure. So i guess, it's not that important.
What matters is that the stucture of the configuration file is ok enough for
the tests to kick in.

vlorentz added inline comments.
swh/search/tests/test_cli.py
37

because it needs to be set to something.

37

Plus, it's in the TEST-NET-1 range, so there's really no harm in it

douardda added inline comments.
swh/search/tests/test_cli.py
37

we are nitpicking, sure, but since this global var is used in one test only, I see no reason to keep it as a global var. It's at best confusing, even if harmless, as you stated below. Harmless for the computer, not for the human reading / maintaining the code IMHO.

This revision is now accepted and ready to land.May 13 2020, 4:02 PM
swh/search/tests/test_cli.py
37

ah yes, it's only used once now. It was used everywhere before. I missed it.

Remove global var and put that definition local to its use

Remove global var and put that definition local to its use

Thx

Build is green

Patch application report for D3151 (id=11185)

Rebasing onto 4dbcbc4e7d...

Current branch diff-target is up to date.
Changes applied before test
commit 225ad532ebeba78791644b0a9e79f2286ca0c46c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed May 13 13:48:01 2020 +0200

    Fix search journal client tests
    
    This:
    - tests: drop the now broken MockedKafkaConsumer test class.
    - migrate the journal client test to pytest
    - fixes the ci [1]
    
    [1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/266/console

See https://jenkins.softwareheritage.org/job/DSEA/job/tests-on-diff/11/ for more details.

This revision was automatically updated to reflect the committed changes.