Page MenuHomeSoftware Heritage

swh-journal: Add tests to the journal publisher using kafka instance
ClosedPublic

Authored by ardumont on Mar 13 2019, 3:31 PM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDJNL240243d53722: tests: Inline the kafka_consumer pytest fixture
rDJNL617b0d4af0cd: bin/install-kafka.sh: Regroup variable together
rDJNL194ce5686cfb: setup.py: Get back to initial formatting
rDJNL71f4f5fec873: test_publisher_kafka: Rename it to be nicer in the diff
rDJNLb70f7d2218a1: tests: Simplify initialization test to use fixtures consistently
rDJNLb8cd821574fa: tests/conftest: Ensure failure message if inexisting kafka install
rDJNL6e6fc773ddba: Makefile.local: Add a target to clean up cache artifacts
rDJNL143f9ce79341: tox: Pass specific environment variable to tox
rDJNL0aa8c5e2c7b4: bin/install-kafka.sh: Fix indentation
rDJNLe49fc8543f5d: Remove unused import
rDJNLbbd1c986478b: test_publisher2: Make the test pass for major object types
rDJNL0e17f6cb5973: README: Explain how to run tests
rDJNL73638b6493ce: setup.py: Use a shell script to ease the kafka development setup
rDJNL13cc5d8d9b01: publisher: Rename tests
rDJNLbdae1997dbfb: publisher: Rename tests 2/2
rDJNL26b9eb434aee: publisher: Add config checks within the publisher
rDJNLf56bb266fbb8: test_publisher2: Fix publisher initialization
rDJNL68fe907f79ec: Remove spurrious print statements
rDJNLf32415142b0d: conftests: Explicit that the broker is changed during fixture init
rDJNL6f3b8e55252d: publisher: Add logger
rDJNL7bfc45ca930a: test_publisher2: Make test work in tox!
rDJNL77cb658ad6b1: test_publisher: Avoid repetition
rDJNL090640f0d27d: test_publisher: Factorize behavior to allow easier new testing
rDJNL5f14837e9120: test_publisher2: Explain why we need the transformation
rDJNLbdaef3fed36b: test_publisher: Use direct instantiation of the in-memory storage
rDJNL3a4238880ba8: test_publisher2: Make the test pass
rDJNLc59be21ee62b: test_publisher2: Progress towards actual publisher test with kafka
rDJNL319f9c135781: tox.ini: Remove unused dependency
rDJNL1bce33eb94b6: test_publisher: Reorganize code
rDJNL5c8779fd3bb6: test_publisher2: Start trying to test with kafka
rDJNLee1ae96cf051: setup.py: Add check on tarball fetch and separate clean up command
rDJNL202cdf1cf5e9: test_publisher: Assert CI is fine with current state
rDJNLcfca42e7f465: tox.ini: Add the pre-download kafka deps step
rDJNL835f953267f1: setup.py: Install kafka alongside tests
rDJNL1cae7e68ca98: Ignore kafka test dependencies
rDJNL8d556630f787: tox.ini: Remove pifpaf
rDJNLfa53bac231c3: Bootstrap publisher tests with kafka instance
rDJNLb66ad089f89b: Install kafka dependencies for test purposes
Summary

Testing the publisher with a real kafka instance through the pytest-kafka dependency.

This also:

  • documents with a script how and what to install in regards to the kafka needed
  • extracts the configuration checks done on the publisher via the cli and push those checks in the publisher directly.
  • those checks are now tested (they helped in the debugging process of those tests)
  • update the readme

Related T1276
Depends on D1280
Depends on D1282

Test Plan

tox

Diff Detail

Repository
rDJNL Journal infrastructure
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Rebase on latest master (and removes the cli adaptation part, D1249)

Make test work for major object types

Build has FAILED

Because flake8 is missing on the job node somehow.

I think i'm done for the main part of the tests.

Now i need to concentrate on removing the part about the target ./setup.py develop.
Well not necessarily removing that part (because that could prove useful for others) but at least not make the ci job rely on it (through tox in the current diff's state).

ardumont retitled this revision from wip: pytest-kafka: ci tryout to swh-journal: Add tests to the journal publisher using kafka instance.Mar 20 2019, 8:55 AM
ardumont edited the summary of this revision. (Show Details)
olasd requested changes to this revision.Mar 20 2019, 10:21 AM

So, if we keep the setup.py snippet to install kafka, I think it should be a completely separate subcommand of setup.py, rather than overriding the default behavior of the develop subcommand, which has a somewhat standard meaning of "install the current module for a development setup".

But I really think those 50 lines of python should just be turned into 5-10 lines of shell script that would be more readable and do the exact same thing.

(disclaimer: I still haven't looked too closely at the actual tests)

So, if we keep the setup.py snippet to install kafka, I think it should be a completely separate subcommand of setup.py, rather than overriding the default behavior of the develop subcommand, which has a somewhat standard meaning of "install the current module for a development setup".

well, it was for that indeed, for the tests to work so for development.

(I don't know yet how to make a subcommand of setup.py, i just started from the pytest-kafka's intial setup.py which does that)

But I really think those 50 lines of python should just be turned into 5-10 lines of shell script that would be more readable and do the exact same thing.

fine by me, i had it in D1280 (prior to removing it) so i'll just take it back for here.

(disclaimer: I still haven't looked too closely at the actual tests)

ack, that's fine ;)

For information, it's just:

  • write to the publisher (as our swh.storage.listener does)
  • publisher reacts, reifies objects and publish them
  • a consumer of the publisher asserts the objects are the expected reified ones
  • README: Explain how to run tests
  • setup.py: Use a shell script to ease the kafka development setup
  • Makefile.local: Add a target to clean up cache artifacts

This will break test as the tox has been adapted to remove the
dependency on setup.py develop. This will be addressed when D1280,
D1282 converge and land.

  • bin/install-kafka.sh: Fix indentation
  • bin/install-kafka.sh: Fix indentation for real this time
  • bin/install-kafka.sh: Fix indentation

After fixing my emacs setup:

;; default indentation setup
(setq-default indent-tabs-mode nil
              tab-width 4)
  • tests/conftest: Ensure failure message if inexisting kafka install
  • tox: Pass specific environment variable to tox
  • setup.py: Get back to initial formatting

Fix pep8 violation

installed back the git-pre-commit hook to avoid that again

ardumont added a subscriber: douardda.
  • bin/install-kafka.sh: Regroup variables together

Following oral discussion with @douardda

  • test_publisher_kafka: Rename test_publisher to make a nicer/smaller diff

Rework commit messages

Aside from that, nothing changed

  • tests: Inline the kafka_consumer pytest fixture
  • tests: Simplify initialization test to use fixtures consistently
This revision was not accepted when it landed; it landed in state Needs Review.Mar 25 2019, 3:53 PM
This revision was automatically updated to reflect the committed changes.