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

refactor: extend the integration testing framework to support multiple languages #2362

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

alecthomas
Copy link
Collaborator

Previously we had full test coverage in each language, so we would just run them all, but as we're building up support for other languages for now, this change allows multiple languages to be supported per-test.

Also cleans up the test harness so there's just a single Run() function whose behaviour can be configured.

@alecthomas alecthomas added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Aug 14, 2024
@alecthomas alecthomas requested review from stuartwdouglas and a team August 14, 2024 08:53
@ftl-robot ftl-robot mentioned this pull request Aug 14, 2024
@alecthomas alecthomas force-pushed the aat/multi-language-integration-tests branch from f9f8385 to 7153aa7 Compare August 14, 2024 08:56
ic.AssertWithRetry(t, func(t testing.TB, ic TestContext) {
_, err := ic.Controller.Status(ic, connect.NewRequest(&ftlv1.StatusRequest{}))
assert.NoError(t, err)
for _, language := range opts.languages {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So my test deploys both of them and has them talk to each other. The go module is used as a kind of source of truth, and the Java module is basically proxying the go module to make sure that both the client invocations and responses are handled correctly.

This change looks like it would not really support testing multiple languages in the same cluster?

Copy link
Collaborator Author

@alecthomas alecthomas Aug 14, 2024

Choose a reason for hiding this comment

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

It will actually support multiple languages in the same cluster, the language name just points to a directory - the directory can contain modules from multiple languages if necessary.

Though that said, there are commonly two types of integration tests we've encountered:

  1. Testing conformance with FTL subsystems.
  2. Testing language interop.

We've found the latter much less common because conformance tests largely encompass language interop too, because if a language complies with the conformance tests it works with other languages by definition.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For example, take TestHttpIngress. This tests that a module correctly conforms to the HTTP ingress "spec". Just dropping a Java module into the testdata/java directory will test this on Java for "free".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar with the PubSub tests, and so on. There are some tests somewhere that deal with encoding types too I think.

@alecthomas alecthomas force-pushed the aat/multi-language-integration-tests branch from 7153aa7 to 0bebc0f Compare August 14, 2024 09:08
…e languages

Previously we had full test coverage in each language, so we would just
run them all, but as we're building up support for other languages for
now, this change allows multiple languages to be supported per-test.
@alecthomas alecthomas force-pushed the aat/multi-language-integration-tests branch from 0bebc0f to f54cb41 Compare August 14, 2024 09:49
Comment on lines +37 to +38
in.WithFTLConfig("ftl-project-dr.toml"),
in.WithoutController(),
Copy link
Member

Choose a reason for hiding this comment

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

oh this is nice! Seems much more intuitive.

@alecthomas alecthomas added this pull request to the merge queue Aug 14, 2024
Merged via the queue into main with commit 71d6cf8 Aug 14, 2024
69 checks passed
@alecthomas alecthomas deleted the aat/multi-language-integration-tests branch August 14, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants