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.

Sign up for GitHub

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

Jump to bottom

ci: fix macos tests #947

Merged
merged 3 commits into from
Oct 9, 2024
Merged

ci: fix macos tests #947

merged 3 commits into from
Oct 9, 2024

Conversation

themilchenko
Copy link
Contributor

@themilchenko themilchenko commented Sep 18, 2024 •
edited
Loading

This PR fixes CI problems appeared in jobs on machine with macOS. There are problems provided below that was fixed.

There was a problem on macOS pipeline that integration tests written on Go fail sometimes with an connection refused error. An instance of Tarantool was unable to start with the specified configuration.

After the patch start options such as WaitStart, ConnectionRetry and RetryTimeout were increased to have enough time for Tarantool startup.

Sometimes CI on macos fails with an error "docker daemon not running".

To avoid it after the patch docker socket is provided in the github env directly.

The test test_t3_instance_names_no_config always fails because of very long path in tmpdir. This test requires socket path with fexed length, otherwise it can cause socket path buffer overflow.

After the patch tmpdir was overwritten to the shorter one.

Closes #931
Closes #912

@themilchenko themilchenko force-pushed the themilchenko/gh-931-fix-macos-ci branch 30 times, most recently from 5144def to ff0e539 Compare September 19, 2024 15:25
@oleg-jukovec
Copy link
Contributor

oleg-jukovec commented Oct 3, 2024 •
edited
Loading

Do ci: overwrite tmpdir in mac integration tests close #959 or not?

@themilchenko
Copy link
Contributor Author

Do ci: overwrite tmpdir in mac integration tests close #959 or not?

959 refers to #959 PR. Which issue did you mean?

@themilchenko themilchenko force-pushed the themilchenko/gh-931-fix-macos-ci branch 4 times, most recently from c69036a to 78c614d Compare October 4, 2024 08:27
@oleg-jukovec
Copy link
Contributor

Do ci: overwrite tmpdir in mac integration tests close #959 or not?

959 refers to #959 PR. Which issue did you mean?

#969

@themilchenko
Copy link
Contributor Author

themilchenko commented Oct 4, 2024 •
edited
Loading

Do ci: overwrite tmpdir in mac integration tests close #959 or not?

959 refers to #959 PR. Which issue did you mean?

#969

I thought that my overriding of tmpdir is just a temporary solution. Don't we need to fix testing logic or something like that, so it should to take into account the length of the socket path?

@oleg-jukovec oleg-jukovec requested a review from psergee October 7, 2024 10:11
psergee
psergee approved these changes Oct 7, 2024
View reviewed changes
Copy link
Collaborator

@psergee psergee left a comment

Choose a reason for hiding this comment

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

Thank you for the patch. lgtm, Just a few minor comments..

.github/workflows/full-ci.yml Show resolved Hide resolved
.github/workflows/full-ci.yml Show resolved Hide resolved
@themilchenko themilchenko force-pushed the themilchenko/gh-931-fix-macos-ci branch 11 times, most recently from 0107fb9 to 1053860 Compare October 9, 2024 09:40
@themilchenko
ci: provide docker socket on macos
56641be 
Sometimes CI on macos fails with an error "docker daemon not running".

To avoid it after the patch docker socket is provided in the github env
directly.

Part of #931
@themilchenko themilchenko force-pushed the themilchenko/gh-931-fix-macos-ci branch 2 times, most recently from 87ab343 to a0738a3 Compare October 9, 2024 12:25
@themilchenko
ci: overwrite tmpdir in mac integration tests
8a378cb 
The test `test_t3_instance_names_no_config` always fails because of very
long path in tmpdir. This test requires socket path with fexed length,
otherwise it can cause socket path buffer overflow.

After the patch tmpdir was overwritten to the shorter one.

Closes #931
@themilchenko themilchenko force-pushed the themilchenko/gh-931-fix-macos-ci branch from a0738a3 to 8a378cb Compare October 9, 2024 12:57
@oleg-jukovec oleg-jukovec merged commit f89bd5b into master Oct 9, 2024
24 checks passed
@oleg-jukovec oleg-jukovec deleted the themilchenko/gh-931-fix-macos-ci branch October 9, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleg-jukovec oleg-jukovec oleg-jukovec approved these changes

@psergee psergee psergee approved these changes

Assignees
No one assigned
Labels
full-ci Enables full ci tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ci: fix failed tests ci: tests don't clean tarantool instances after running on macOS
3 participants
@themilchenko @oleg-jukovec @psergee