Skip to content
This repository was archived by the owner on Sep 17, 2024. It is now read-only.

IndexAggregation unit tests#100

Closed
oluiscabral wants to merge 3 commits into
ARK-Builders:mainfrom
oluiscabral:test--index-aggregation
Closed

IndexAggregation unit tests#100
oluiscabral wants to merge 3 commits into
ARK-Builders:mainfrom
oluiscabral:test--index-aggregation

Conversation

@oluiscabral

@oluiscabral oluiscabral commented Sep 29, 2023

Copy link
Copy Markdown
Contributor

Just as note. This PR still a draft.

It has to test those methods yet:

  • getResource(id: ResourceId);
  • allPaths();
  • getPath(id: ResourceId);
  • updateAll().

@kirillt kirillt requested a review from mdrlzy September 30, 2023 04:21
@oluiscabral

Copy link
Copy Markdown
Contributor Author

Just as note. This PR still a draft.

It has to test those methods yet:

  • getResource(id: ResourceId);
  • allPaths();
  • getPath(id: ResourceId);
  • updateAll().

Remaining tests created

@ARK-Builders ARK-Builders deleted a comment from sonarqubecloud Bot Oct 2, 2023
fun testAllResources() = runBlocking {
val expected = shards.map { it.allResources() }.toList().reduce { acc, map -> acc + map }
val allResources = indexAggregation.allResources()
assertEquals(9, allResources.size) // 3 resources per shard, so 3 shards * 3 resources

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test situation when resource sets from different shards intersect.

E.g.
shard1: a, b, c
shard2: c, d, e
shard3: e, f, g

Aggregation: a, b, c, d, e, f, g

}

@Test
fun testAllResourcesEmpty() = runBlocking {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call it something like "empty aggregation contains no resources".

@Test
fun testGetResourceNotFound() = runBlocking {
val indexAggregation = IndexAggregation(setOf())
val result = indexAggregation.getResource(ResourceId(0, 123456789))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ids with zero size are forbidden actually. Let's create dedicated test case for this somewhere (not really relevant to aggregation).

About this test case, let's create a constant for an "arbitrary id which is not in the index".

@kirillt

kirillt commented Dec 20, 2023

Copy link
Copy Markdown
Member

@tuancoltech would be great if you could take over this PR. We need to re-create it though, because it originates in @oluiscabral fork.

@tuancoltech

Copy link
Copy Markdown
Contributor

@tuancoltech would be great if you could take over this PR. We need to re-create it though, because it originates in @oluiscabral fork.

@kirillt Sure, I'll take it over.

@kirillt kirillt mentioned this pull request Feb 9, 2024
2 tasks
@kirillt

kirillt commented Feb 9, 2024

Copy link
Copy Markdown
Member

Superseded by:

@kirillt kirillt closed this Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants