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

[cache_unittest] Add direct implementation testing on ordering, pruning (backport #678) #689

Merged
merged 3 commits into from
May 28, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented May 24, 2024

Towards #676
This changes nothing pertaining to the implementation. It only adds testing.
Some of it is implementation-specific, but towards the ends of performance checks.

From #676 (comment)

  1. It needs to hold an arbitrary number of entries, because we don't know the rate at which we will get data.
  2. The items needs to be in sorted order by timestamp, from the newest entry to the oldest.
  3. New items need to be inserted into arbitrary positions, as timestamps may come in out-of-order.
  4. Duplicates shouldn't be allowed.
  5. Old items need to be pruned out.

To answer them:

  1. There appears to be no limit on size. Not sure how to test this in a bounded fashion.
  2. This was not tested. Now explicitly tested via ImplSortedDescendingUniqueEntries
  3. This was not tested. Now explicitly tested via ImplSortedDescendingUniqueEntries
  4. This was already tested, but is also explicitly tested via ImplSortedDescendingUniqueEntries
  5. This was not tested (I believe). Now explicitly tested via ImplSortedDescendingUniqueEntries
    This is an automatic backport of pull request [cache_unittest] Add direct implementation testing on ordering, pruning #678 done by Mergify.

…ng (#678)

* [cache_unittest] Add direct implementation testing on ordering, pruning

* do getAllItems() approach

* Return a reference instead.

* mark getAllItems as internal

* Fix warning on Windows.

Co-authored-by: Chris Lalancette <clalancette@gmail.com>
(cherry picked from commit a17a2bf)

# Conflicts:
#	tf2/include/tf2/time_cache.h
@mergify mergify bot added the conflicts label May 24, 2024
Copy link
Contributor Author

mergify bot commented May 24, 2024

Cherry-pick of a17a2bf has failed:

On branch mergify/bp/iron/pr-678
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit a17a2bf8.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   tf2/src/cache.cpp
	modified:   tf2/test/cache_unittest.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   tf2/include/tf2/time_cache.h

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde
Copy link
Contributor

ahcorde commented May 28, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
@ahcorde ahcorde merged commit 34549c5 into iron May 28, 2024
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/iron/pr-678 branch May 28, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants