Created Integration Tests for SPM #5608 #5636
Conversation
Signed-off-by: kunal Dugar <yoyokvunal@gmail.com>
kunal-511
force-pushed
the
intergration_test_for_SPM
branch
from
69d93b5
to
f7ae46e
Compare
| run: sudo curl -L "https://github.com/docker/compose/releases/download/v2.11.2/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose && sudo chmod +x /usr/local/bin/docker-compose | ||
|
|
||
| - name: Start Docker Compose services | ||
| run: docker-compose -f docker-compose/monitor/docker-compose.yml up -d |
There was a problem hiding this comment.
we don't write integration tests like that. There has to be a way to run integration test locally, for debugging. In your approach you can only run it via GitHub workflow, which prevents debugging. The workflow should just run a script.
| - name: v2 | ||
| binary: jaeger | ||
| skip_sampling: true | ||
| - name: v1 |
There was a problem hiding this comment.
please undo all whitespace changes, they are not required for your PR
| QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }} | ||
|
|
||
| - name: Set up Docker Compose | ||
| run: sudo curl -L "https://github.com/docker/compose/releases/download/v2.11.2/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose && sudo chmod +x /usr/local/bin/docker-compose |
There was a problem hiding this comment.
docker compose is already available, we don't install it
| # Check if Prometheus is accessible | ||
| curl -f http://localhost:9090 || exit 1 | ||
| # Check if Grafana is accessible | ||
| curl -f http://localhost:3000 || exit 1 |
There was a problem hiding this comment.
The objective of e2e test is to verify that Jaeger components are fulfilling the expected function. In case of SPM, we can query the internal API that returns the time series results to the UI and validate that there are data points in those results.
| - name: Run smoke tests | ||
| run: | | ||
| # Wait for services to be up | ||
| sleep 30 |
There was a problem hiding this comment.
we never do it like this - it introduces a fixed delay to the CI that is almost never needed. Instead we query the service in a loop with a small sleep between iterations, this way as soon as the service is available we exit the loop. You can see example of this in wait_for_storage in scripts/es-integration-test.sh
|
@yurishkuro I have added another commit with all issues being addressed. Please have a look |
…tions included, removed the docker compose, removed fixed delay. Signed-off-by: kunal Dugar <yoyokvunal@gmail.com>
kunal-511
force-pushed
the
intergration_test_for_SPM
branch
from
0819838
to
60b7b6d
Compare
There was a problem hiding this comment.
@kunal-511 are you sure you have pushed the right changes ? I don't see the workflow running a script. Also please remove the extra white space. Please address all the comments on the PR.
kunal-511
force-pushed
the
intergration_test_for_SPM
branch
from
02973c7
to
54ee0f7
Compare
Signed-off-by: kunal Dugar <yoyokvunal@gmail.com>
kunal-511
force-pushed
the
intergration_test_for_SPM
branch
from
d7832d7
to
bf2b3c0
Compare
|
solved in #5640 |
My First PR.
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test