Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: pass in time parameter to prevent flakes#11023

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
f0ssel merged 6 commits intomainfromf0ssel/time-test-logs
Dec 4, 2023

Conversation

f0ssel
Copy link
Contributor

@f0sself0ssel commentedDec 4, 2023
edited
Loading

Closes#10600

I'm trying to understand why these tests flake sometimes, like here:https://github.com/coder/coder/actions/runs/6805426561/job/18504925666#step:5:428

Here's what I have so far:

  • The tests failed very close to midnight:insights_internal_test.go:162: now: 2023-11-08 23:59:56.781794 +0000 UTC m=+7.293516543
    • I noticed the monotonic clock reading,and upon further reading about it I think it's reasonable to strip this when dealing with parsed times since we want our operations to match the presentation time. Nvm, time.Parse already has this stripped so that won't do anything...
    • Some tests fail with time not being an even hour, which I believe could be caused by a difference in wall vs monotonic clock. We now think this is because we create a newtime.Now() inside the function, and with processing lag this can be significant enough to change the time returned.
  • I've added more logging aroundLocation and other times in the tests, as well as more verbose error responses so we know what times it thinks it got. My hunch is that all of these tests fail when it runs on a CI machine that has different time settings than we are used to running in.

With the logs we hopefully have a better shot next time this happens.

Update:

  • After speaking with@mafredri we now pass thenow time into the function. This is so if we are processing the test very slowly (like in CI) the drift can cause flakes when on a time barrier (end of hour, end of day, etc).

@@ -550,6 +550,9 @@ func parseInsightsStartAndEndTime(ctx context.Context, rw http.ResponseWriter, s
{"end_time", endTimeString, &endTime},
} {
t, err := time.Parse(insightsTimeLayout, qp.value)
// strip monotonic clock reading
// so presentation time and arithmetic operations are consistent
t = t.Round(0)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to strip the monotonic clock here since we're parsing a string. Monotonic clock should only be present whentime.Now() is used. 🤔

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yeah I just learned that after making this :P

@f0sself0ssel requested a review frommafredriDecember 4, 2023 17:06
@f0sself0ssel changed the titlefix: strip monotonic clock from parsing timefix: pass in time parameter to prevent flakesDec 4, 2023
@f0sself0ssel merged commit1e6ea61 intomainDec 4, 2023
@f0sself0ssel deleted the f0ssel/time-test-logs branchDecember 4, 2023 17:20
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsDec 4, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@kylecarbskylecarbskylecarbs approved these changes

@mtojekmtojekAwaiting requested review from mtojek

@mafredrimafredriAwaiting requested review from mafredri

Assignees

@f0sself0ssel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flake: Test_parseInsightsStartAndEndTime
4 participants
@f0ssel@mafredri@kylecarbs@deansheather

[8]ページ先頭

©2009-2025 Movatter.jp