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

https://github.com/benbjohnson/clock is EOL/archived. #1135

Closed
dnozay opened this issue Nov 17, 2023 · 1 comment · Fixed by #1154
Closed

https://github.com/benbjohnson/clock is EOL/archived. #1135

dnozay opened this issue Nov 17, 2023 · 1 comment · Fixed by #1154
Assignees
Labels

Comments

@dnozay
Copy link

dnozay commented Nov 17, 2023

Describe the bug
https://github.com/benbjohnson/clock is archived.

To Reproduce
N/A, we can code scanning tools in-house that complain about archived repo (considered EOL)

Expected behavior
N/A, code scanning shouldn't complain about go.mod.

Additional context
based on https://github.com/benbjohnson/clock/network maybe https://github.com/itbasis/go-clock would be a good in-place replacement.

╰─ go mod graph | grep github.com/benbjohnson/clock
go.uber.org/fx@v1.20.0 github.com/benbjohnson/clock@v1.3.0
go.uber.org/zap@v1.24.0 github.com/benbjohnson/clock@v1.1.0
go.uber.org/zap@v1.18.1 github.com/benbjohnson/clock@v1.1.0
go.uber.org/zap@v1.19.0 github.com/benbjohnson/clock@v1.1.0
@dnozay
Copy link
Author

dnozay commented Nov 17, 2023

see also uber-go/zap#1349

@sywhang sywhang self-assigned this Nov 21, 2023
abhinav added a commit to abhinav/fx that referenced this issue Feb 7, 2024
abhinav added a commit to abhinav/fx that referenced this issue Feb 9, 2024
benbjohnson/clock has been archived and no longer maintained.
Removing that dependency is desirable.

This change adds a mock clock adapted from Zap's mock clock
(uber-go/zap#1349), but with operations specific to Fx's needs.

There are two areas that require more scrutiny
because of divergence from Zap's mock clock:

WithTimeout requires us to implement a custom `context.Context`
so that it can report context.DeadlineExceeded when it's time.
We cannot just use `context.WithCancelCause` here because the cause
for a context failure is considered different from `ctx.Err`,
so `ctx.Err` would still report `context.Canceled` for a timeout.

When testing that a sleep behaves as expected, there's a data race
between the `Sleep` and the `Add` that progresses time.
To resolve this, this change adds an `AwaitScheduled` method
that blocks until there are operations scheduled for the future.
This is done by using a `sync.Cond`.
This gives us a way to wait until the sleep is scheduled
before we progress time.

This change also had to update Zap to pick up the release with the
custom clock to drop the benbjohnson/clock dependency from the go.mod
completely.

Refs uber-go/zap#1349
Resolves uber-go#1135
abhinav added a commit to abhinav/fx that referenced this issue Feb 12, 2024
benbjohnson/clock has been archived and no longer maintained.
Removing that dependency is desirable.

This change adds a mock clock adapted from Zap's mock clock
(uber-go/zap#1349), but with operations specific to Fx's needs.

There are two areas that require more scrutiny
because of divergence from Zap's mock clock:

WithTimeout requires us to implement a custom `context.Context`
so that it can report context.DeadlineExceeded when it's time.
We cannot just use `context.WithCancelCause` here because the cause
for a context failure is considered different from `ctx.Err`,
so `ctx.Err` would still report `context.Canceled` for a timeout.

When testing that a sleep behaves as expected, there's a data race
between the `Sleep` and the `Add` that progresses time.
To resolve this, this change adds an `AwaitScheduled` method
that blocks until there are operations scheduled for the future.
This is done by using a `sync.Cond`.
This gives us a way to wait until the sleep is scheduled
before we progress time.

This change also had to update Zap to pick up the release with the
custom clock to drop the benbjohnson/clock dependency from the go.mod
completely.

Refs uber-go/zap#1349
Resolves uber-go#1135
abhinav added a commit that referenced this issue Feb 14, 2024
benbjohnson/clock has been archived and no longer maintained.
Removing that dependency is desirable.

This change adds a mock clock adapted from Zap's mock clock
(uber-go/zap#1349), but with operations specific to Fx's needs.

There are two areas that require more scrutiny
because of divergence from Zap's mock clock:

WithTimeout requires us to implement a custom `context.Context`
so that it can report context.DeadlineExceeded when it's time.
We cannot just use `context.WithCancelCause` here because the cause
for a context failure is considered different from `ctx.Err`,
so `ctx.Err` would still report `context.Canceled` for a timeout.

When testing that a sleep behaves as expected, there's a data race
between the `Sleep` and the `Add` that progresses time.
To resolve this, this change adds an `AwaitScheduled` method
that blocks until there are operations scheduled for the future.
This is done by using a `sync.Cond`.
This gives us a way to wait until the sleep is scheduled
before we progress time.

This change also had to update Zap to pick up the release with the
custom clock to drop the benbjohnson/clock dependency from the go.mod
completely.

Refs uber-go/zap#1349
Resolves #1135

---------

Co-authored-by: Jacob Oaks <jacoboaks.8@gmail.com>
Co-authored-by: Sung Yoon Whang <sungyoonwhang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants