Skip to content

Move object storage server tests out of tree#100

Merged
sharpener6 merged 3 commits intofinos:mainfrom
gxuu:object_server_test_improvement
May 4, 2025
Merged

Move object storage server tests out of tree#100
sharpener6 merged 3 commits intofinos:mainfrom
gxuu:object_server_test_improvement

Conversation

@gxuu
Copy link
Contributor

@gxuu gxuu commented May 1, 2025

The title of this PR is misleading. See comment for what it does. It is basically the second round of enhancement to the object storage server that is less important than the last one.

  • object storage server tests were in the source tree, while the correct place of putting tests is tests/ directory. This changes the tests location.

  • test_scaler_object test was not self-contained. This changes it, and make the test self-contained, so we can treat it as a unit test.

  • Dependency CapnProto is now a project dependency instead of a component dependency. Rationale: Dependencies of a necessary component of a project should be treated as a project dependency; Dependencies of an optional component of a project (such as tests) should be placed in sub-directory. Since scheduler will also use CapnProto, it is considered as a necessary dependency.

  • Project directory is now an include directory to every source file. C++ sources should always write "scaler/..." to get the header files they need.

TODO:

  • Fix linter.yml behavior for tests as tests are now moved to another position.

  • Remove redundant tests in scaler/object_storage.

  • Logging support to object storage server, currently the error message is raw.

  • Fix the packaging logic to the following: all build should not contain tests and examples; sdist build should not contain any binary; and wheel build should contain dependencies' binaries. Currently the packaging logic does not conform to these rules.

  • Further simplify the test_scaler_object, the connecting logic should also be moved to global setup.

  • Potentially change cmake script to get SO from system directory.

@gxuu gxuu marked this pull request as draft May 1, 2025 04:53
@gxuu gxuu force-pushed the object_server_test_improvement branch from 981f20f to 8fde754 Compare May 1, 2025 17:22
@gxuu
Copy link
Contributor Author

gxuu commented May 1, 2025

Done:

  • object storage server tests were in the source tree, while the correct
    place of putting tests is tests/ directory. This changes the tests
    location.

  • test_scaler_object test was not self-contained. This changes it, and
    make the test self-contained, so we can treat it as a unit test.

  • Dependency CapnProto is now a project dependency instead of a
    component dependency. Rationale: Dependencies of a necessary component
    of a project should be treated as a project dependency; Dependencies
    of an optional component of a project (such as tests) should be placed
    in sub-directory. Since scheduler will also use CapnProto, it is
    considered as a necessary dependency.

  • Project directory is now an include directory to every source file.
    C++ sources should always write "scaler/..." to get the header files
    they need.

  • Fix linter.yml behavior for tests as tests are now moved to another
    position.

  • Remove redundant tests in scaler/object_storage.

  • Further simplify the test_scaler_object, the connecting logic should
    also be moved to global setup. (CANCELLED. Test cannot access
    variable defined in global setup).

TODO:

  • Logging support to object storage server, currently the error message
    is raw.

  • Fix the packaging logic to the following: all build should not
    contain tests and examples; sdist build should not contain any
    binary; and wheel build should contain dependencies' binaries.
    Currently the packaging logic does not conform to these rules.

  • Potentially change cmake script to get SO from system directory.

Signed-off-by: gxu georgexu420@gmail.com

- object storage server tests were in the source tree, while the correct
  place of putting tests is tests/ directory. This changes the tests
  location.

- test_scaler_object test was not self-contained. This changes it, and
  make the test self-contained, so we can treat it as a unit test.

- Dependency CapnProto is now a project dependency instead of a
  component dependency. Rationale: Dependencies of a necessary component
  of a project should be treated as a project dependency; Dependencies
  of an optional component of a project (such as tests) should be placed
  in sub-directory. Since scheduler will also use CapnProto, it is
  considered as a necessary dependency.

- Project directory is now an include directory to every source file.
  C++ sources should always write "scaler/..." to get the header files
  they need.

- Fix linter.yml behavior for tests as tests are now moved to another
   position.

- Remove redundant tests in scaler/object_storage.

- Further simplify the test_scaler_object, the connecting logic should
  also be moved to global setup. (CANCELLED. Test cannot access
  variable defined in global setup).

- Fix the packaging logic to the following: all build should not
  contain tests and examples; sdist build should not contain any
  binary; and wheel build should contain dependencies' binaries.
  Currently the packaging logic does not conform to these rules.

- Potentially change cmake script to get SO from system directory.
  (CANCELLED, we should let tools to handle it. Copy SO will not work
  because SO will also have dependencies).

// TODO:
//  - Logging support to object storage server, currently the error message
//    is raw. (Logging support will be move to another PR and will be
//    discussed)

Signed-off-by: gxu <georgexu420@gmail.com>
@gxuu gxuu force-pushed the object_server_test_improvement branch from 2c6637e to 0fd9f7f Compare May 2, 2025 01:23
@gxuu
Copy link
Contributor Author

gxuu commented May 2, 2025

Move object storage server tests out of tree

- object storage server tests were in the source tree, while the correct
  place of putting tests is tests/ directory. This changes the tests
  location.

- test_scaler_object test was not self-contained. This changes it, and
  make the test self-contained, so we can treat it as a unit test.

- Dependency CapnProto is now a project dependency instead of a
  component dependency. Rationale: Dependencies of a necessary component
  of a project should be treated as a project dependency; Dependencies
  of an optional component of a project (such as tests) should be placed
  in sub-directory. Since scheduler will also use CapnProto, it is
  considered as a necessary dependency.

- Project directory is now an include directory to every source file.
  C++ sources should always write "scaler/..." to get the header files
  they need.

- Fix linter.yml behavior for tests as tests are now moved to another
   position.

- Remove redundant tests in scaler/object_storage.

- Further simplify the test_scaler_object, the connecting logic should
  also be moved to global setup. (CANCELLED. Test cannot access
  variable defined in global setup).

- Fix the packaging logic to the following: all build should not
  contain tests and examples; sdist build should not contain any
  binary; and wheel build should contain dependencies' binaries.
  Currently the packaging logic does not conform to these rules.

- Potentially change cmake script to get SO from system directory.
  (CANCELLED, we should let tools to handle it. Copy SO will not work
  because SO will also have dependencies).

// TODO:
//  - Logging support to object storage server, currently the error message
//    is raw. (Logging support will be move to another PR and will be
//    discussed)

Signed-off-by: gxu <georgexu420@gmail.com>

@gxuu gxuu marked this pull request as ready for review May 2, 2025 01:32
@gxuu
Copy link
Contributor Author

gxuu commented May 2, 2025

ready for review

Signed-off-by: gxu <georgexu420@gmail.com>
@gxuu gxuu changed the title WIP Move object storage server tests out of tree Move object storage server tests out of tree May 2, 2025
Signed-off-by: gxu <georgexu420@gmail.com>
@sharpener6 sharpener6 merged commit b5b9a5e into finos:main May 4, 2025
5 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.

2 participants