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

test: cleanup remote_timeline_client tests #5013

Merged
merged 6 commits into from
Aug 17, 2023

Conversation

koivunej
Copy link
Member

@koivunej koivunej commented Aug 16, 2023

I will have to change these as I change remote_timeline_client api in #4938. So a bit of cleanup, handle my comments which were just resolved during initial review.

Cleanup:

  • use unwrap in tests instead of mixed ? and unwrap
  • use Handle instead of &'static Reactor to make the RemoteTimelineClient more natural
  • use arrays in tests
  • use plain #[tokio::test]

@koivunej koivunej requested review from a team as code owners August 16, 2023 18:35
@koivunej koivunej requested review from knizhnik and hlinnaka and removed request for a team August 16, 2023 18:35
@github-actions
Copy link

github-actions bot commented Aug 16, 2023

1596 tests run: 1521 passed, 0 failed, 75 skipped (full report)


The comment gets automatically updated with the latest test results
f71daeb at 2023-08-16T19:15:25.210Z :recycle:

@koivunej koivunej enabled auto-merge (squash) August 17, 2023 13:29
@problame
Copy link
Contributor

use plain #[tokio::test]

just to understand: this is equivalent to what we had before because tokio::test uses current-thread runtime by default, right?

@koivunej koivunej merged commit 67af241 into main Aug 17, 2023
@koivunej koivunej deleted the remote_timeline_client_testing_cleanup branch August 17, 2023 16:27
@koivunej
Copy link
Member Author

use plain #[tokio::test]

just to understand: this is equivalent to what we had before because tokio::test uses current-thread runtime by default, right?

It is no longer leaking memory to get a &Runtime, so it'd say it is better.

When non-awaitpoint code is being executed, nothing else moves.

koivunej added a commit that referenced this pull request Oct 26, 2023
…#4938)

Implement a new `struct Layer` abstraction which manages downloadness
internally, requiring no LayerMap locking or rewriting to download or
evict providing a property "you have a layer, you can read it". The new
`struct Layer` provides ability to keep the file resident via a RAII
structure for new layers which still need to be uploaded. Previous
solution solved this `RemoteTimelineClient::wait_completion` which lead
to bugs like #5639. Evicting or the final local deletion after garbage
collection is done using Arc'd value `Drop`.

With a single `struct Layer` the closed open ended `trait Layer`, `trait
PersistentLayer` and `struct RemoteLayer` are removed following noting
that compaction could be simplified by simply not using any of the
traits in between: #4839.

The new `struct Layer` is a preliminary to remove
`Timeline::layer_removal_cs` documented in #4745.

Preliminaries: #4936, #4937, #5013, #5014, #5022, #5033, #5044, #5058,
#5059, #5061, #5074, #5103, epic #5172, #5645, #5649. Related split off:
#5057, #5134.
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