[POC] CSGHub server integration testing framework

[Yiling-J]

Starting and Stopping the Test Environment

Note: The test server requires Docker and an internet connection (if the images have not been pulled yet) to run correctly, as we use Testcontainers to start various services and components.

Setting up the test environment for integration testing is simple. Follow the steps below:

ctx := context.TODO()
env, err := testinfra.StartTestEnv()
defer func() { _ = env.Shutdown(ctx) }()

However, before proceeding with writing your tests, please read the following explanation to fully understand what happens in these three lines of code. A lot is happening behind the scenes.

When testinfra.StartTestEnv() is called, the following actions are performed in sequence:

1. Load the configuration file: The common/config/test.toml configuration file is loaded. This config is used during integration tests.
2. Create a test PostgreSQL database: A PostgreSQL database is created on a random port using test containers. The database configuration in the test config is updated accordingly.
3. Start the Gitaly server: A Gitaly server is started using test containers. The configuration used for Gitaly is either tests/gitaly.toml or tests/gitaly_github.toml (used when running on GitHub). Please see the comment in the testinfra.go explaining why two config files are required. The Gitaly server configuration is updated once the container is started.
4. Start the Temporal test server: A local Temporal test server is started using the temporaltest package. The workflow endpoint config is also updated. By default, the Temporal test server uses a random namespace to avoid conflicts, but we force the registration of the default namespace to ensure tests run.
5. Start the in-memory S3 server: A local in-memory S3 server is started using GoFakeS3. This server is used with the MinIO Go SDK for testing LFS (Large File Storage) functionality. The S3 configuration is updated accordingly.
6. Start the Redis server: A Redis server is started using test containers, and the Redis endpoint configuration is updated.
7. Start the CSGHub user server: The CSGHub user server and its workflows are started.
8. Start the CSGHub dataset viewer server: The CSGHub dataset viewer server and its workflows are started.
9. Start the CSGHub main API server: The CSGHub main API server and its workflows are started.

That’s all. All docker containers are started using Testcontainers, so all data will be removed after test done.Note that not all services are started by default. For example, the NATS server or runner server is not started. If you need to test functionality related to these services, be sure to add them to the environment startup function.

After the test environment is started, always defer the call to env.Shutdown(ctx) to ensure all resources are properly cleaned up.

Writing Tests

There are two example test files:

• api/model_test.go: This tests CRUD operations for models and Git-related functionality. Since the model, dataset, space, and code all share the same repo/Git code, you can consider this file to also test dataset, space, and code-related features.
• api/dataset_viewer_test.go: This file tests the Temporal workflows for the dataset viewer server.

The test code in these files clearly demonstrates how to test the API, Git, and workflows. There’s no need to repeat this here.

One important thing to remember is that for all integration tests, you should add the following snippet at the beginning of your test function:

if testing.Short() {
	t.Skip("skipping integration test")
}

This allows you to differentiate between unit tests and integration tests.

MR Summary:

The summary is added by @codegpt.

This Merge Request introduces an integration testing framework for the CSGHub server, utilizing Docker and Testcontainers to set up various services. Key updates include:

1. Added a Shutdown method to the GracefulServer for proper server shutdown.
2. Refactored server initialization to improve error handling and modularity.
3. Enhanced integration tests for dataset viewer and model CRUD operations.
4. Introduced a test environment setup in testinfra for consistent testing.
5. Improved configuration loading and management for tests.

Overall, this MR significantly enhances the testing capabilities of the CSGHub server.

[starship-github]

Linter Issue Report

During the code review, a list issues were found. These issues could affect the code quality, maintainability, and consistency. Below is the detailed Linter issue report:

common/tests/testutils.go

Lint Issue: Undefined: testcontainers

Code Context and Location:
The issue is located at line 92, column 3. Here is the snippet of the code around the issue:

...
\treuse := testcontainers.CustomizeRequestOption(
\t\tfunc(req *testcontainers.GenericContainerRequest) error {
\t\t\treq.Reuse = true
\t\t\treq.Name = cname
\t\t\treturn nil
\t\t},
\t)
...

Actionable Suggestions:
The linter has identified an issue where the testcontainers package is undefined. This could be due to the package not being imported or a typo in the package name.

To resolve this issue, please ensure that the testcontainers package is correctly imported at the top of your file. If it is already imported, check for any spelling or case-sensitivity errors in the package name.

Here is an example of how to import the package:

import (
    "github.com/testcontainers/testcontainers-go"
)

Please make sure to replace the import path with the correct one if it's different.

tests/testinfra/testinfra.go

Lint Issue: Undefined: workflowservice

• Code Context and Location:

err = nsclient.Register(ctx, &workflowservice.RegisterNamespaceRequest{
	Namespace:                        "default",
	WorkflowExecutionRetentionPeriod: &durationpb.Duration{Seconds: 1000000000},
})

This issue is located at line 200, column 32.

• Actionable Suggestions:
  It seems like the workflowservice package is not imported or not defined in the current scope. Please check if you have imported the package correctly. If not, add the import statement at the top of your file. If the package is in a different directory, make sure to provide the correct path in the import statement. If workflowservice is a variable or function, ensure it is defined before this line of code.

---

Please make the suggested changes to improve the code quality.

[starship-github]

• Comments:
  • The error handling for testinfra.StartTestEnv() is placed after the function call, which will cause a panic if err is not nil. This should be checked immediately after the call.
• Suggestions:

  ctx := context.Background()
  env, err := testinfra.StartTestEnv()
  if err != nil {
      panic(err)
  }
  defer func() { _ = env.Shutdown(ctx) }()
  

[starship-github]

• Comments:
  • The test for model detail exposes user email, which could lead to privacy issues.
• Suggestions:

  // Consider removing or masking the user email in the response
  require.NotEmpty(t, gjson.GetBytes(b, "data.user.email").String())
  

[starship-github]

• Comments:
  • The gitCommitAndPush function does not handle errors correctly; it should return an error if the command fails.

[starship-github]

• Comments:
  • The Shutdown method does not handle errors from t.temporalServer.Stop() properly; it should check for errors.
• Suggestions:

  if err := t.temporalServer.Stop(); err != nil {
      return err
  }
  

[starship-github]

• Comments:
  • The use of sync.OnceValues is incorrect; it should be sync.Once instead. This could lead to unexpected behavior.
• Suggestions:

  once = &sync.Once{}
  

[starship-github]

• Comments:
  • The CreateTestDB function should handle errors from CreateTestDB instead of ignoring them. This could lead to silent failures.
• Suggestions:

  db, err := CreateTestDB("csghub_test")
  if err != nil {
      log.Fatalf("failed to create test DB: %v", err)
  }
  

[starship-github]

• Comments:
  • The GetDB function does not handle the case where defaultDB is nil, which could lead to a nil pointer dereference.
• Suggestions:

  if defaultDB == nil {
      return nil, errors.New("defaultDB is not initialized")
  }
  return defaultDB
  

[starship-github]

• Comments:
  • The configuration for cfg.MultiSync.SaasAPIDomain should be validated to ensure it is not empty before use.
• Suggestions:

  if cfg.MultiSync.SaasAPIDomain == "" {
      t.Fatal("SaasAPIDomain must be set")
  }
  

[starship-github]

Possible Issues And Suggestions:

• Line 57 in tests/api/dataset_viewer_test.go

  • Comments:
    • The gitClone function does not check if the directory already exists before cloning, which could lead to errors.
  • Suggestions:

    if _, err := os.Stat(dir); os.IsNotExist(err) {
        err = gitClone(url, dir)
    }
    

• api/router/api.go

  • Comments:
    • The RunServer function panics on error, which is not a good practice for production code. Consider returning the error instead.
  • Suggestions:

    if err != nil {
        return err
    }
    

• user/workflow/worker.go

  • Comments:
    • The logger import path is incorrect; it should be "log/slog" instead of "log".
  • Suggestions:

    import (
        "log/slog" // Ensure this is the correct import path
    )
    

• api/workflow/activity/sync_as_client.go

  • Comments:
    • The check for an empty apiDomain returns nil, which may lead to unexpected behavior. Consider logging or handling this case differently.
  • Suggestions:

    if apiDomain == "" {
        slog.Warn("SaasAPIDomain is empty, skipping sync")
        return nil
    }
    

• tests/testserver/testserver.go

  • Comments:
    • The error handling in the StartTestServer function could be improved by using a more descriptive error message.
  • Suggestions:

    panic(fmt.Errorf("failed to start test environment: %w", err))
    

• Consider adding more error handling throughout the test cases to ensure robustness.

• Ensure that sensitive information is not exposed in test outputs or logs.

MR Evaluation:

This feature is still under test, evaluation are given by AI and might be inaccurate.

After evaluation, the code changes in the Merge Request get score: 100.

Tips

CodeReview Commands (invoked as MR or PR comments)

• @codegpt /review to trigger an code review.
• @codegpt /evaluate to trigger code evaluation process.
• @codegpt /describe to regenerate the summary of the MR.
• @codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
• @codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

• Review comments: Directly reply to a review comment made by StarShip.
  Example:
  • @codegpt How to fix this bug?
• Files and specific lines of code (under the "Files changed" tab):
  Tag @codegpt in a new review comment at the desired location with your query.
  Examples:
  • @codegpt generate unit testing code for this code snippet.

Note: Be mindful of the bot's finite context window.
It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.
For a focused discussion, use review comments to chat about specific files and their changes, instead of using the MR/PR comments.

CodeReview Documentation and Community

• Visit our Documentation
  for detailed information on how to use Starship CodeReview.

About Us:

Visit the OpenCSG StarShip website for the Dashboard and detailed information on CodeReview, CodeGen, and other StarShip modules.