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

Drop test_db #788

Merged
merged 1 commit into from
Jun 24, 2022
Merged

Drop test_db #788

merged 1 commit into from
Jun 24, 2022

Conversation

AlekSi
Copy link
Member

@AlekSi AlekSi commented Jun 23, 2022

We are not using it much now, but it makes task env-up much slower.

Checklist

  • Tests are added for new functionality or fixed bugs.
  • task all passes.

See CONTRIBUTING.md for more details.

We are not using it much now, but it makes `task env-up` much slower.
@AlekSi AlekSi added the code/chore Code maintenance improvements label Jun 23, 2022
@AlekSi AlekSi added this to the v0.5.0 milestone Jun 23, 2022
@AlekSi AlekSi self-assigned this Jun 23, 2022
Comment on lines -324 to -340
var portsCheckError error

wg.Add(1)
go func() {
defer wg.Done()

logger.Info("Waiting for port 37017 to be up...")
portsCheckError = waitForPort(portsCtx, 37017)
}()

wg.Add(1)
go func() {
defer wg.Done()

logger.Info("Waiting for port 5432 to be up...")
portsCheckError = waitForPostgresPort(portsCtx, 5432)
}()
Copy link
Member Author

@AlekSi AlekSi Jun 23, 2022

Choose a reason for hiding this comment

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

Among other things, there was a data race – two goroutines assigned to the same variable portsCheckError

@codecov
Copy link

codecov bot commented Jun 23, 2022

Codecov Report

Merging #788 (4306563) into main (712e940) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #788   +/-   ##
=======================================
  Coverage   59.27%   59.27%           
=======================================
  Files         212      212           
  Lines        9348     9348           
=======================================
  Hits         5541     5541           
  Misses       3163     3163           
  Partials      644      644           
Flag Coverage Δ
FerretDB 62.67% <ø> (ø)
MongoDB 7.22% <ø> (ø)
integration 62.77% <ø> (ø)
unit 22.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@AlekSi AlekSi marked this pull request as ready for review June 24, 2022 05:33
@AlekSi AlekSi requested a review from rumyantseva as a code owner June 24, 2022 05:33
@AlekSi AlekSi modified the milestones: v0.5.0, v0.4.0 Jun 24, 2022
@AlekSi AlekSi requested review from a team, seeforschauer and noisersup and removed request for a team June 24, 2022 05:34
@AlekSi AlekSi enabled auto-merge (squash) June 24, 2022 05:34
Copy link
Member

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

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

LGTM

@AlekSi
Copy link
Member Author

AlekSi commented Jun 24, 2022

We never had -race in go run ./cmd/envtool/main.go command; otherwise, we would notice that problem sooner

@AlekSi AlekSi merged commit a963d6a into FerretDB:main Jun 24, 2022
@AlekSi AlekSi deleted the remove-values branch June 24, 2022 11:16
AlekSi added a commit to AlekSi/FerretDB that referenced this pull request Jun 24, 2022
We are not using it much now, but it makes `task env-up` much slower.
@AlekSi AlekSi mentioned this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants