-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SAM-232: Testing improvements - use docker compose to run 3rd party services #462
Draft
eapearson
wants to merge
147
commits into
develop
Choose a base branch
from
feature-SAM-232
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… and zookeeper, tools for starting and stopping, and functional documentation [SAM-232]
…ependencies not covered by docker compose [SAM-232]
…services, add enhanced script and makefile support for tests [SAM-232]
…rts in some files [SAM-232]
…r and all related code and configuration [SAM-232]
…rate the testing changes [SAM-232]
…proved coverage reporting support [SAM-232]
…>release_tag [SAM-238]
…separate workflows; keeps each one simpler [SAM-238]
…separate workflows; keeps each one simpler [SAM-238]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SAM-232: Testing improvements - use docker compose to run 3rd party services
Sample Service tests require a set of 3rd party (and KBase) services. Integration tests work directly against some services and also implicitly use in tests of sample service code.
These 3rd party services must be downloaded and installed manually. The test code contains code whose purpose is simply to control these services.
The primary purpose of this set of changes is to replace this usage of host-run service binaries with docker-container-run services.
This brings some advantages:
simplifies setup required before tests can be run
simplifies need for test support code to run hosted service binaries
produces more reliable (repeatable, deterministic) tests
In addition, a few other changes were required in order to more efficiently implement the above:
simplified GHA Workflow
reorganize test directory
replace
sed
with Python script using Jinja2 for test config generationBenefits
Simplified test setup of required services
Before these changes, a developer would need to obtain and install binaries and required support libraries for:
arangodb
kafka
zookeeper
mongodb
After the changes in this PR all of these dependencies are configured by and launched from a single docker compose file. There is no need to install anything, other than Docker, which is, with near certainty, surely already installed and used on the developer's machine.
This obviates the need to either document how to install and use these programs, or to provide tooling and service wrapper code to automate such.
Simplified test support
Each of the service dependencies required some code in order to configure, launch, and control the services.
After these changes, all of that support code has been removed. Configuration and control of the services is through a single docker compose file and through docker compose itself.
Improved Reliability
Since the running of tests relies upon manually installed service binaries, test running may differ between different developer machines, and between the developer machine and the GitHub Action platform (Ubuntu).
By using docker containers to run the test services for both development and GHA testing, the comparability of the test services across testing environments will be much closer. There still may be differences introduced by different docker versions, resources allocated to docker, and host-docker volume mounting technology.
Overview of Changes
remove code to manage test services
add docker compose for test services
update tests to use containerized test services
reorganize tests
create scripts and make tasks to automate test setup and execution
Use
jinja2
rather thansed
for test config generationsimplified GHA workflow
Remove code to manage test services
Test services were previously run directly from test code. This required modules to manage the services, including configuration, starting, and stopping.
Files:
REMOVED test/arango_controller.py
REMOVED test/kafka-controller.py
REMOVED test/mongo-controller.py
Re-Organize test files
authjars, wsjars moved into test/bin
all test code moved under test/lib
all tests moved under test/lib/specs
all test support code under test/lib/test_support
Improve test config template
REMOVED test/lib/test.cfg.example for usage by sed
ADDED test/lib/test.cfg.template in jinja2 format added
ADDED test/lib/test_support/compile-template.py script which creates test.cfg from teset.cfg.template
Add docker compose for test services
ADDED test/docker-compose-services.yml
Makefile
ADDED start-test-services
ADDED stop-test-services
update tests to use containerized test services
The primary changes within test code is to accommodate usage of the containerized services, which is itself primarily enabled through the usage of pytest fixtures.
Most of these changes involve replacing fixtures which returned a controller to simply returning the port for the service.
Test utilities which expect information from fixtures needed to be updated to work with the new or modified fixtures.
Another set of changes, made ad-hoc as encountered, was to consolidate test configuration, fixtures, assertion utilities, and general utilities into files separate from the tests themselves, to facilitate re-use.
Add utilities to properly wait for services to start up
Already present in the system, the utlity
test/lib/test_support/wait_for.py
was extended to provide a function to monitor an instance of mongodb for readiness.This utility is used to for the test automation task to pause before running tests until arangodb and mongodb are fully started.
reorganize tests
A few changes were made to reorganize test code, configuration, and support libraries in order to make refactoring tests easier. Particularly, when refactoring tests it is very convenient to simply move tests out of the testing directory in order to isolate one or more test files. When tests are mixed with configuration, utility and other files, such test isolation is more difficult.
All test code lives in test/lib, which is a nice parallel to service code which lives in lib.
All tests reside in test/lib/specs, and all test support code lives in test/lib/test_support. This separate helps facilitate test setup as well as re-use of test utilities.
The pytest setup file
conftest.py
was created intest/lib/specs
, from where it is automatically detected by purest. This is the supported location to place test fixtures. Alongside the fixtures are a few functions and to support the fixtures. Some fixtures remain in test files.A new
bin
directory was added to contain the pre-installedauthjars
andwsjars
jar files, and in which to install the KBase jars during test setup. It had previously (in an earlier iteration of this PR) contained the mongo binary as well.The top level test directory is reserved for test configuration, including the test config file, the test services docker compose file, and the coverage configuration.
Add scripts and
make
tasks to automate test setup and executionAll test automation is rooted in
make
tasks. Some make tasks call shell scripts, others python scripts, and others invoke other executables such ascoverage
andpytest
.make test-setup
,test/scripts/test-setup.sh
ensure host dependencies are installed
installs python dependencies and creates pipenv virtual environment
install jars
install test config from template
make start-test-services
make stop-test-services
make test
Orchestrates all test tasks, each of which is also a make task, including:
run mypy type verification test
start test services
wait for arango and mongodb to be started
run tests
stop test services
generate coverage reports
make test-sdkless
make test-types
simplified GHA workflow
In addition to being run by developers on their host machine, tests are also invoked by the GitHub Action workflow(s). Thus the GHA workflow needed modification.
When I turned to the GHA workflows, I found quite a bit of unnecessary complexity. To whit, 15 separate workflow and script files existing. They can be replaced by a one.
During the PR review we should see if there are specific issues the various scripts were intending to solve that the single GHA workflow does not.
Much of the simplification is achieved by using "standard" (i.e. commonly used, supported) GHA actions rather than custom scripts.
This change brings the following improvements:
that it is easy to reason about, especially since all triggering conditions are easily discoverable
it exposes much less code to maintain
it uses standard GHA actions to perform tasks rather than own own bespoke scripts
it uses the same make tasks as local develop does, reducing duplication and increasing reliability
Use
jinja2
rather thansed
for test config generationA configuration file, though reduced in size with this set of changes, is required in order for tests to run. This configuration file is generated from a template, with specific environment variables substituted for placeholders with the same name.
The current technique is to use
sed
to substitute specific environment variables into snippets of text within the template.This technique suffers problems:
it is not a common technique within KBase
it does not use a standard templating format
the implementation of
sed
varies between host operating systemsI was triggered to make this change when finding that
sed
on macOS and on Linux (Ubuntu) differs enough that it can cause unexpected test failure. (Specifically, the usage of sed on macOS utilizes the -i/-I in-place editing option, while it appears that Ubuntu does not.)The replacement is a simple Python script
lib/cli/compile-template.py
which usesjinja2
. jinja2 is widely used at KBase, and was already a Python dependency of the project.This change also allows us to drop
sed
as a project requirement.improve test code
test/lib/test_support/constants.py
; this helps reduce repetition