Skip to content

test: add backend integration tests for server and analytics plugins#79

Open
calvarjorge wants to merge 5 commits intomainfrom
jorge.calvar/create_appkit_backend_integration_tests
Open

test: add backend integration tests for server and analytics plugins#79
calvarjorge wants to merge 5 commits intomainfrom
jorge.calvar/create_appkit_backend_integration_tests

Conversation

@calvarjorge
Copy link
Contributor

  • Create backend integration tests for AppKit
  • Add reusable createTestServer() utility for integration tests
  • Move existing server integration tests to src/tests/integration/
  • Add new integration tests for analytics and server plugins in this folder

…ugins

Signed-off-by: Jorge Calvar <jorge.calvar@databricks.com>
Signed-off-by: Jorge Calvar <jorge.calvar@databricks.com>
@pkosiec pkosiec self-requested a review February 3, 2026 13:22
Copy link
Member

Choose a reason for hiding this comment

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

What was the reason of refactoring the server.integration.test.ts?

Personally I like to keep tests close to the actual code - maybe we should revisit the previous structure?

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 usually followed the pattern of having unit tests next to the actual code and integration tests in their own package, since they're usually run together and have common componentes. For instance, a typical flow in the CI would be to run all unit tests and, if those pass (or in parallel to that job), have a job run the integration test package.

Happy to have them in their respective dir, since we only have two plugins, and if needed we can re-visit later.

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to test it similarly to the deleted server.integration.test.ts? That is, we could add the analytics.integration.test.ts close to the analytics plugin, and maybe we don't need the actual test server? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I undid the changes of server.integration.test.ts. My reasoning was that since the server is a common component of all integration tests, I would be nice to abstract it to a helper func.

Resolve conflict by keeping main's simpler server.integration.test.ts
@calvarjorge calvarjorge force-pushed the jorge.calvar/create_appkit_backend_integration_tests branch from a0fb1ad to bd9fb65 Compare February 5, 2026 13:46
Address PR review feedback:
- Delete centralized integration test infrastructure (src/tests/integration/)
- Keep original simple server.integration.test.ts (from main)
- Add analytics.integration.test.ts colocated with analytics plugin
- Use existing mockServiceContext pattern for simpler test setup
@calvarjorge calvarjorge force-pushed the jorge.calvar/create_appkit_backend_integration_tests branch from bd9fb65 to 31dd57b Compare February 5, 2026 14:07
@calvarjorge calvarjorge requested a review from pkosiec February 5, 2026 14:12
server = app.server.getServer();
baseUrl = `http://127.0.0.1:${TEST_PORT}`;

await new Promise((resolve) => setTimeout(resolve, 100));
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this for? 😅

Copy link
Contributor Author

@calvarjorge calvarjorge Feb 6, 2026

Choose a reason for hiding this comment

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

Not needed - was meant to give the server a bit of time to start up, but I see tests run fine without it. Removed it.

Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants