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

ci: Add testing on macOS #268

Merged
merged 7 commits into from
Mar 24, 2023
Merged

ci: Add testing on macOS #268

merged 7 commits into from
Mar 24, 2023

Conversation

better0fdead
Copy link

@better0fdead better0fdead commented Feb 7, 2023

Added macOS testing into ci.

  • [not needed] Tests
  • [Added] Changelog
  • [not needed] Documentation

Closes #157

@better0fdead better0fdead changed the title Better0fdead/gh 157 macos ci: Add testing on macOS Feb 7, 2023
@better0fdead better0fdead marked this pull request as draft February 7, 2023 13:27
@better0fdead better0fdead force-pushed the better0fdead/gh-157-macos branch 3 times, most recently from 4c3af3b to 81c8daf Compare February 7, 2023 13:48
@better0fdead better0fdead marked this pull request as ready for review February 7, 2023 14:01
Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

I would like to see a motivation in commit messages for test fixes. In the future it will be difficult to understand why it is necessary.

.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Outdated Show resolved Hide resolved
.github/workflows/testing.yml Outdated Show resolved Hide resolved
.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Outdated Show resolved Hide resolved
.github/workflows/testing.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
.github/workflows/testing.yml Show resolved Hide resolved
.github/workflows/testing.yml Outdated Show resolved Hide resolved
connection_pool/connection_pool_test.go Show resolved Hide resolved
connection_pool/connection_pool_test.go Show resolved Hide resolved
queue/queue_test.go Outdated Show resolved Hide resolved
@better0fdead better0fdead force-pushed the better0fdead/gh-157-macos branch 2 times, most recently from 157256d to 4a82ce9 Compare February 7, 2023 15:34
@better0fdead better0fdead force-pushed the better0fdead/gh-157-macos branch 6 times, most recently from e755d1f to 31effa0 Compare March 16, 2023 09:37
Added macOS testing into ci.
Updated actions to versions with node16 runtime.

Closes #157
Added workaround for macOs-12 testrace.
Without it testrace sends SIGABRT after start.

Part of #157
Changed checkTimeout time in test.
The reason is that by default it updates the connects once every 1 second.
And it doesn't have time to update for some reason in the case of a test
 in the same second.
So set the update to half the time.

Part of #157
Increased sleep duration. Added more checks.

Part of #157
Changed action version from 2 to 3.

Part of #157
Copy link
Collaborator

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

I see a flaky test on macOS:

2023-03-16T14:18:05.7459090Z --- FAIL: TestReconnect (0.62s)
2023-03-16T14:18:05.7567240Z     multi_test.go:108: conn has incorrect addr: 127.0.0.1:3013 after disconnect server1

I think you need to make a change in TestReconnect similar too:

b0956f3

  1. Stop the tarantool instance instead of just conn.Close() to avoid auto-reconnects.
  2. Ensure that the connection is gone from a connection pool (in a loop with N retries with a little delay between iterations).
  3. Restart the instance.
  4. Ensure that the connection is available for requests (in a loop with N retries with a little delay between iterations).

@better0fdead better0fdead force-pushed the better0fdead/gh-157-macos branch 3 times, most recently from d474a64 to 34a606e Compare March 24, 2023 11:22
if !ok {
break
}
multiConn.getState()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
multiConn.getState()

Rewrote TestReconnect.
Instead of just conn.Close() used stop the tarantool instance to
avoid auto-reconnect. Restarted it after. Added checks.

Part of #157
Changed sleep duration time in test.
The reason is that ttl time is set to 5 seconds in test.
And it doesn't have time for some reason in the case of a test in
the same time.
So set the sleep duration to 10 seconds.

Part of #157
@oleg-jukovec oleg-jukovec merged commit 888d74d into master Mar 24, 2023
@oleg-jukovec oleg-jukovec deleted the better0fdead/gh-157-macos branch March 24, 2023 13:02
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.

Run tests on macOS
3 participants