Add integration tests #39
Conversation
|
@vdye @derrickstolee - Opening this as a draft PR, as I'd like to add some more commands and an actions workflow before merging. However, I'm sure there are lots of areas for improvement, so wanted to open up the floor for feedback as soon as possible. |
ldennington
force-pushed
the
integration-tests
branch
from
dbc9056 to
385b958
Compare
There was a problem hiding this comment.
Good start to the new test category. I'm confused about the boundaries between the test classes.
Another thought: is there value in having a single Cucumber project, but splitting e2e or integration into different categories? Then the Makefile could run the different categories for different build targets. Our workflows could be organized to run unit and integration tests in one build, and e2e tests in another build (if we need to parallelize them).
There was a problem hiding this comment.
Thanks for working on this! In addition to what I've already noted in individual comments, one general recommendation I have would be to keep the architecture a bit simpler as you add more tests/steps. The deeper the call stack is for a given operation, the harder it is to understand and subsequently modify or extend its behavior.
In the context of these tests, one way to avoid having lots of wrapper layers (functions or classes) would be to pull more logic into the BundleServerWorld or even the step_definitions. That needs to be balanced with avoiding code duplication, although even then it's okay to have a little repeated code if it makes for better readability.
ldennington
force-pushed
the
integration-tests
branch
2 times, most recently
from
9128704 to
24632ca
Compare
|
I believe all the previous feedback has been addressed, either directly or via another suggested refactor. Please share any additional suggestions you have for improvement on the new changes (command tests, the refactors for sharing code, CI, etc.) |
ldennington
force-pushed
the
integration-tests
branch
from
24632ca to
ee9c3ab
Compare
There was a problem hiding this comment.
Awesome tests! The .features are descriptive and thoroughly verify on-disk behavior, and the shared infrastructure between e2e and integration is quite elegant.
I had a few super minor nits + some suggestions for possible consolidation (although I'm fine deferring that to a separate issue/future change). Otherwise, 🚢.
| private root: string | ||
| private remote: RemoteRepo | undefined | ||
|
|
||
| root: string |
There was a problem hiding this comment.
It's used in the remote.ts step definition to run Git commands, for example in Given('{int} commits are pushed to the remote branch {string}'). Sorry about the lack of link - I don't think permalinks are suppported in PRs, or if they are I have not figured out how to obtain them 😬.
Returning to the topic at hand, I could change it to be private again and add a getter if that would be better though!
|
@ldennington I've also unmarked |
ldennington
force-pushed
the
integration-tests
branch
from
ee9c3ab to
2cc8fbc
Compare
Update necessary components of existing e2e tests to prepare for new integration test suite. This includes: 1. Moving package-related files (package-lock.json, package.json, and tsconfig.json) out of the 'e2e' directory and into 'test' and updating package.json to not be e2e-test specific. This will allow both suites to be combined into a single Typescript package in upcoming patches.i 2. Moving certain classes currently in the e2e 'step_definitions', 'support', and 'classes' directories that will be needed by integration tests into a new 'shared' directory. This led to notable refactorings to implement the shared 'BundleServerWorld' class as a parent and a new 'E2EBundleServerWorld' class as a child. A similar 'IntegrationBundleServerWorld' class will be added in a future patch. 3. Updating the 'require' statement in the e2e tests' config to identify the correct 'features' directories with the new architecture. This indicates to Cucumber that it should look in multiple directories for features. The VSCode settings have been updated to recognize these directories as well. Signed-off-by: Lessley Dennington <[email protected]>
Add integration tests for the following commands, which represent core bundle server functionality: 1. init 2. delete 3. update 4. start 5. stop Signed-off-by: Lessley Dennington <[email protected]>
Add basic integration tests for daemon start/stop functionality. Signed-off-by: Lessley Dennington <[email protected]>
Add three new Makefile targets: 1. 'test' - runs the project's unit tests 2. 'test-integration' - installs the required dependencies for and runs the project's integration tests, following the example set in [1] 3. 'test-all' - runs all project test suites (unit, integration, and end-to-end) 1: a6ff664 Signed-off-by: Lessley Dennington <[email protected]>
Update existing test.yml workflow to run integration tests.
ldennington
force-pushed
the
integration-tests
branch
from
2cc8fbc to
511ed32
Compare
|
Branch protection rules have been updated! |
This series adds integration tests, using the example set with the end-to-end tests in #33.
The first commit refactors certain components of the end-to-end tests to prepare for the new integration suite. The second commit adds the basic infrastructure and tests for the core bundle server commands. The third adds tests for basic daemon functionality. The fourth updates the Makefile to add targets to run unit tests, integration tests, and all tests. The fifth adds applicable tests from the integration suite to CI.