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

Tests for tcp.echo, tls.echo and http.kafka.sync examples #126

Merged
merged 6 commits into from
May 7, 2024

Conversation

attilakreiner
Copy link
Contributor

@attilakreiner attilakreiner commented Apr 18, 2024

This change introduces automated tests for tcp.echo, tls.echo and http.kafka.sync examples.

Fixes #125

@attilakreiner attilakreiner force-pushed the kind-poc branch 30 times, most recently from 1168753 to c6c1b91 Compare April 25, 2024 07:54
@attilakreiner attilakreiner force-pushed the kind-poc branch 6 times, most recently from 9f67235 to 94cf174 Compare April 29, 2024 08:33
@attilakreiner attilakreiner force-pushed the kind-poc branch 3 times, most recently from 6bc2ad9 to 11b41e4 Compare April 29, 2024 12:59
@attilakreiner attilakreiner changed the title WIP Tests for tcp.echo, tls.echo and http.kafka.sync examples Apr 29, 2024
Comment on lines 36 to 39
if [ -z "$CORRELATION_ID" ]; then
echo ❌
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to repeat the failure output here when the test fails early?
In the general case, where it is possible to fail early in N different ways, this would be repeated N times.

For example, if each part is a function, then the execution invokes each function in turn, skipping over functions after early failure, falling through to output success or failure once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually my preference is to repeat those two lines (echo x; exit 1) whenever it makes sense to exit prematurely as opposed to introduce complex control flow with functions and such. This way we can keep a linear execution flow with the only exception of exiting early in case of fatal errors. I hope it's OK this way.

.github/workflows/test.yaml Outdated Show resolved Hide resolved
@jfallows jfallows linked an issue May 1, 2024 that may be closed by this pull request
Comment on lines +56 to +63
- name: Install kcat
working-directory: /tmp
run: |
set -x
curl -L -o kcat https://github.com/attilakreiner/kcat/releases/download/1.7.1/kcat-linux-$(arch)
chmod +x kcat
sudo mv kcat /usr/local/bin
kcat 2>&1 | grep "Version 1.7.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use apt package kafkacat instead of this manual install?

https://github.com/edenhill/kcat?tab=readme-ov-file#install

Copy link
Contributor Author

@attilakreiner attilakreiner May 2, 2024

Choose a reason for hiding this comment

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

The apt package kafkacat installs kafkacat version 1.6.0 that means it's lagging ca. one year behind the latest release. Even if we installed that we'd need to do some manual steps (e.g. renaming the executable from kafkacat to kcat) to keep the test in sync with the README and executable on other systems (e.g. on a Mac which has the latest kcat installed with brew).

Copy link
Contributor Author

@attilakreiner attilakreiner May 2, 2024

Choose a reason for hiding this comment

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

Actually I tested it, and that approach runs the test green, too. This is the alternative configuration:

      - name: Install apt packages
        uses: awalsh128/cache-apt-pkgs-action@latest
        with:
          packages: kafkacat
          version: 1.0
      - name: Setup kcat
        run: |
          sudo ln -s /usr/bin/kafkacat /usr/local/bin/kcat
          kcat 2>&1 | grep "Version 1.6.0"

Pls LMK which one you prefer.

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 reverted the apt install commit as discussed, so going back to manual installation of kcat, as the decision was to install the latest version.

@attilakreiner attilakreiner force-pushed the kind-poc branch 4 times, most recently from c8515a3 to 002c6b8 Compare May 2, 2024 07:59
This reverts commit fa4c1fa.
@vordimous vordimous merged commit 0277a61 into aklivity:main May 7, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automate tls.echo scenario acceptance tests
3 participants