Add faketime tests #4843
Closed
Add faketime tests #4843
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request has been linked to Shortcut Story #43955: Write simulation test for single-process write serializability of array fragments. |
|
Windows build are failing, looking into it. |
|
That's because |
|
Let's make it linux-only right now (I believe |
teo-tsirpanis
requested changes
Apr 2, 2024
There was a problem hiding this comment.
- As noted before,
libfaketimedoes not work on Windows. According to previous comments, we should run this in CI on Linux, but you might need to find another way to do it instead of anif(CMAKE_SYSTEM_NAME STREQUAL "Linux"); there is no reason to prevent it from running on macOS. - The vcpkg port should be upstreamed eventually. Right now it's not suitable for upstreaming and requires the following changes:
- Because the upstream library does not use CMake, the CMake packages and targets exported by the port should have an
unofficialprefix.- This should be addressed in this PR to enable us to smoothly move to the upstream port when it becomes available.
- To reduce the port's maintenance burden, the port should use the upstream Makefile-based build system. For our own convenience we can add a CMake config file to enable consuming with
find_package. You can see how I did the latter for libmagic in [libmagic] Add CMake config. microsoft/vcpkg#35274.- This does not need to be done now. I might do it later one day when I have some free time. If you are interested in doing it to get more familiar with authoring vcpkg ports let me know and I will leave it to you.
- Because the upstream library does not use CMake, the CMake packages and targets exported by the port should have an
teo-tsirpanis
approved these changes
Apr 3, 2024
Co-authored-by: Theodore Tsirpanis <[email protected]>
Co-authored-by: Theodore Tsirpanis <[email protected]>
--- TYPE: NO_HISTORY DESC: Unstatus compressors directory.
Addresses #4570 (review) --- TYPE: NO_HISTORY
--- TYPE: NO_HISTORY DESC: Update dev history with 2.21.0 and 2.21.1 content.
--- TYPE: NO_HISTORY DESC: Bump dev version to 2.23.0.
[SC-43870](https://app.shortcut.com/tiledb-inc/story/43870/remove-azure-warning-when-not-using-azure-credentials) This PR changes the Azure VFS to fail with an explicit error if an operation on the Azure VFS was attempted, but an endpoint to Azure Blob Storage could not be determined based on the `vfs.azure.storage_account_name` and `vfs.azure.blob_endpoint` configuration options. The previous behavior was to emit a warning, which happened at the initialization of the VFS class if Azure is enabled, regardless of whether Azure is being actually used. That warning is no longer emitted. --- TYPE: IMPROVEMENT DESC: Improve diagnostics when an Azure endpoint is not configured.
[SC-36911](https://app.shortcut.com/tiledb-inc/story/36911/remove-enable-vcpkg-options-from-cmake-command-line-and-bootstrap-scripts) This PR changes the severity of the message when we configure with `-DTILEDB_VCPKG=OFF` from a `DEPRECATION` to a `FATAL_ERROR`. This means that using the legacy `ExternalProject`-based mechanism for acquiring dependencies is no longer supported. The `ExternalProject` modules have become dead code and will be removed in a subsequent PR. --- TYPE: BUILD DESC: Vcpkg is always enabled; turning the `TILEDB_VCPKG` option off is no longer supported and fails.
As per title, more tests in tiledb_unit are adapted to run on the REST CI runner. After this PR is merged we'll have 160 tests ported to REST-CI runner, of which 143 will be running and passing and 17 will be disabled until the issues found and logged here:https://app.shortcut.com/tiledb-inc/story/40489/issues-found-while-running-tiledb-unit-core-tests-against-rest-cloud-server are fixed. --- TYPE: NO_HISTORY DESC: Port more tests to REST CI
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup` command. This PR updates the port containing the command to adapt to the new behavior. The change will be upstreamed with microsoft/vcpkg#38017. Validated locally. For ease of review, the first commit copies the existing port as-is, and the second commit updates it. Fixes #4857 Fixes TileDB-Inc/TileDB-CSharp#405 Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83 --- TYPE: NO_HISTORY
We added more tests to REST, which ends up increasing the number of tests running against HDFS. --- TYPE: NO_HISTORY DESC: Increase HDFS timeout in nightlies.
[SC-42590](https://app.shortcut.com/tiledb-inc/story/42590) [SC-42830](https://app.shortcut.com/tiledb-inc/story/42830) This PR adds support for [Google Cloud service account impersonation](https://cloud.google.com/docs/authentication/use-service-account-impersonation) to the GCS VFS. This is enabled with a new config option `vfs.gcs.impersonate_service_account` that also supports chained impersonation if given a comma-separated list of service accounts. Because we were using some older APIs to configure the GCS client and these don't support impersonation, I also switched to the newer APIs. The migration process was straightforward. For ease of review, you are suggested to look at each commit individually. --- TYPE: CONFIG DESC: Add `vfs.gcs.impersonate_service_account` option that specifies a service account to impersonate, or a comma-separated list for chained impersonation. --- TYPE: IMPROVEMENT DESC: Stop using deprecated Google Cloud SDK APIs. --------- Co-authored-by: Isaiah Norton <[email protected]>
[SC-44515](https://app.shortcut.com/tiledb-inc/story/44515/support-specifying-gcp-account-credentials-as-a-config-option) --- TYPE: CONFIG DESC: Add `vfs.gcs.service_account_credential` config option that specifies a Google Cloud service account credential JSON string. --- TYPE: CONFIG DESC: Add `vfs.gcs.external_account_credential` config option that specifies a Google Cloud Workload Identity Federation credential JSON string.
…4865) sc-44928 We were getting failures while testing Query v3 , and specifically when setting up the REST-CI environment for it, because the `rest.use_refactored_array_open_and_query_submit` config we were setting was not getting propagated to the REST server. After debugging it seems that Config serialization code was calling `param_values()` method to get the config variables to set in Cap'n'proto, which is not taking into account environment variables. This PR also moves `param_values()` to private to prevent other classes from similar mistakes. `ConfiIter` is the only class that still uses it and I only saw that class being used in `S3Parameters::load_headers`. It's worth investigating further if that's a problem or not. --- TYPE: BUG DESC: Config serialization should take into account environment variables
…#4867) Add _internal_ `Config` option, `sm.memory.tracker.reporter.wait_time_ms`, to toggle thread block time in `MemoryTrackerManager::run`. --- TYPE: NO_HISTORY DESC: Add internal Config option to toggle thread wait time in the MemoryTracker.
Moves generators in the beginning of tests. Flow of execution and clean up can get unclear/messy when GENERATORS and SECTIONs are interleaved. Testing: I re-ran the REST-CI 3 times and it passed all of them. Not sure the problem is fixed, but for sure it didn't make it worse. sc-42190 --- TYPE: NO_HISTORY DESC: Stabilize REST CI
This PR adds the foundations for testing connectivity of TileDB to the real clouds by adding a new workflow named `test-cloud-e2e`. Currently only Azure with Microsoft Entra ID authentication is provided. Authentication happens [with OpenID Connect](https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-azure). A new environment was created that has read-only access to the `core` container of the `tiledbci` storage account, and we test that the file `test.txt` in that container exists. The test is configured with environment variables, making it reusable for subsequent tests with other cloud providers. --- TYPE: NO_HISTORY --------- Co-authored-by: Theodore Tsirpanis <[email protected]>
Merged
|
I had issues with rebasing/merging dev branch so I created a new PR: #4883 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces timing tests that run with mocked time. During the test all calls to acquire timestamp, either by using posix
time()or modern C++std::chrono::clock::now()are mocked and should return the same timestamp. This way we should be able to test sub-millisecond writes/reads.This PR adds:
These tests are supposed to fail on TileDB version 2.20.0:
https://github.com/dudoslav/TileDB/actions/runs/8535232007/job/23381209269#step:14:97
TYPE: NO_HISTORY
DESC: Add faketime tests