Skip to content

Conversation

@arvid-e
Copy link
Contributor

@arvid-e arvid-e commented Nov 11, 2025

@arvid-e arvid-e requested a review from yuki-takei November 18, 2025 08:07
$project: {
_id: 0,
d: '$_id',
c: '$count',
Copy link
Contributor

Choose a reason for hiding this comment

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

key が d, c ってわかりにくい

action: { $in: Object.values(ActivityLogActions) },
timestamp: {
$gte: startDate,
$lt: new Date(),
Copy link
Contributor

Choose a reason for hiding this comment

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

クエリキャッシュのパフォーマンスが低下する可能性

start date も end date も、hour, minute, second は 0 で初期化する

date: '$timestamp',
timezone: 'Z',
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

クエリキャッシュのパフォーマンスが低下する可能性

$dateTrunc を使えないか?
https://www.mongodb.com/docs/manual/reference/operator/aggregation/datetrunc/


expect(aggregateMockFn).toHaveBeenCalledTimes(1);
expect(mockExec).toHaveBeenCalledTimes(1);
expect(result).toEqual(mockDbOutput);
Copy link
Contributor

Choose a reason for hiding this comment

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

テストが「本質的なテスト」になっていない

「本質的なテスト」とは?:

  • arrange, act, assert パターンに於いて、arrange で act, assert に都合のいいデータや mock を作っていないか?
    • 本質的でないテストでは、act, assert に都合のいいデータや mock を作っている
  • 実装を利用したテストになっているか?
    • 本質的でないテストでは、実装が変わってデグレしたとしてもテストが成功してしまう

現状では、runAggregationPipeline が呼ばれたかどうかだけをテストしており、実際の buildPipeline がどのように集計しどのような集計結果になるべきかをテストしていない

@arvid-e arvid-e force-pushed the feat/172653-contribution-graph branch from c659c00 to 601f4b0 Compare November 19, 2025 09:49
@arvid-e arvid-e force-pushed the feat/172666-aggregation-pipeline-contribution-activity branch from 69e977b to 6f72cc6 Compare November 19, 2025 09:51
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