Skip to content
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

Add integration tests for gofail #69

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

henrybear327
Copy link
Contributor

@henrybear327 henrybear327 commented May 14, 2024

Add integration test for the gofail library.

The idea is to make sure features, such as the one introduced in #65 will not have regression in future changes.

@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch from 7805958 to 577c5e3 Compare May 14, 2024 20:54
@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch 4 times, most recently from 8195e54 to fd562c1 Compare May 15, 2024 20:08
@henrybear327 henrybear327 changed the title [DO-NOT-MERGE] Fix execution of failpoint should not block deactivation integration test Add integration tests for gofail May 15, 2024
@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch from fd562c1 to c0dea6d Compare May 17, 2024 12:15
@henrybear327
Copy link
Contributor Author

henrybear327 commented May 17, 2024

/cc @ahrtr the code here is now rebased!

@k8s-ci-robot k8s-ci-robot requested a review from ahrtr May 17, 2024 12:17
@ahrtr
Copy link
Member

ahrtr commented May 22, 2024

Please take a look & fix the workflow failure.

@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch 4 times, most recently from b2652ba to f3490e0 Compare May 22, 2024 18:33
Makefile Outdated Show resolved Hide resolved
integration/sleep/go.mod Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented May 22, 2024

cc @ArkaSaha30 PTAL

Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
Co-authored-by: Benjamin Wang <benjamin.wang@broadcom.com>
@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch from b79fe8a to 9f57df4 Compare May 23, 2024 12:58
Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
@henrybear327 henrybear327 force-pushed the improvement/Execution-of-failpoint-should-not-block-deactivation_integration_test branch from 7f17e8b to 7ca32e4 Compare May 23, 2024 13:20
Comment on lines +1 to +20
#!/usr/bin/env bash

set -euo pipefail

GOFAIL_BINARY="$(pwd)/gofail"

cd $(dirname $0)

pushd failpoints
$GOFAIL_BINARY enable
popd

go build -o integration_test_sleep .

pushd failpoints
$GOFAIL_BINARY disable
popd

./integration_test_sleep
rm integration_test_sleep
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to follow https://github.com/etcd-io/etcd/blob/main/tests/robustness/makefile.mk to get most of the work included in a makefile.mk included in integration

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try to follow the Makefile style in the Robustness test and replicate it here! :)

- uses: actions/checkout@v4
- name: tests
run: |
make run-integration-test

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per makefile it should be make run-all-integration-test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, thanks ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants