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

test: handle everything with go test #137

Merged
merged 4 commits into from
Feb 14, 2022
Merged

test: handle everything with go test #137

merged 4 commits into from
Feb 14, 2022

Conversation

DifferentialOrange
Copy link
Member

@DifferentialOrange DifferentialOrange commented Jan 17, 2022 •
edited
Loading

Before this patch, it was required to set up test tarantool processes
manually (and also handle their dependencies, like making working dir).
You can see an example in CI scripts.

This patch introduces go helpers for starting a tarantool process and
validating Tarantool version. Helpers are based on os/exec calls.
Retries to connect test tarantool instance handled explicitly,
see #136.

Setup scripts are reworked to use environment variables to configure
box.cfg. Listen port is set in the end of script so it is possible
to connect only if every other thing was set up already.

Every test is reworked to start a tarantool process (or processes) in
TestMain before test run.

To run tests, install all dependencies with running root folder

./deps.sh

and then run

go clean -testcache && go test ./... -v -p 1

Flag -p 1 means no parallel runs. If you run tests without this flag,
several test tarantool instances will try to bind the same port,
resulting in run fail.

Closes #107

@DifferentialOrange DifferentialOrange mentioned this pull request Jan 17, 2022
Merged
Base automatically changed from DifferentialOrange/gh-134-fix-mod-deps to master January 31, 2022 08:29
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from 510da5f to 45def97 Compare January 31, 2022 08:30
LeonidVas
LeonidVas requested changes Jan 31, 2022
View reviewed changes
Copy link
Collaborator

@LeonidVas LeonidVas left a comment

Choose a reason for hiding this comment

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

Hi! Thank you for the patch. It is generally ok. But I have several questions.

  • AFAIU we do not clear artifacts after testing is over, is this true
  • See comments bellow

config.lua Outdated Show resolved Hide resolved
multi/config.lua Outdated Show resolved Hide resolved
multi/multi_test.go Outdated Show resolved Hide resolved
multi/multi_test.go Outdated Show resolved Hide resolved
test_helpers/main.go Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from 45def97 to 413bef4 Compare January 31, 2022 15:51
@DifferentialOrange
Copy link
Member Author

DifferentialOrange commented Jan 31, 2022 •
edited
Loading

  • AFAIU we do not clear artifacts after testing is over, is this true

Yes, I'll change it

UPD: clean work_dir after run

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from 413bef4 to bc75d65 Compare January 31, 2022 16:21
LeonidVas
LeonidVas approved these changes Feb 1, 2022
View reviewed changes
Copy link
Collaborator

@LeonidVas LeonidVas left a comment

Choose a reason for hiding this comment

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

Hi! I left several minor comments.
I approved this PR and suggest to ping a second reviewer.

multi/config.lua Outdated Show resolved Hide resolved
queue/deps.sh Outdated Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch 3 times, most recently from b6f8631 to 8314aa6 Compare February 1, 2022 16:06
@DifferentialOrange
Copy link
Member Author

I've added support for go test ./..., following #139 (comment) . Now test_helpers also support Tarantool version verification (so go test ./... will be possible with Tarantool 1.10 and UUID tests).

@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch 2 times, most recently from d708c20 to 909543e Compare February 1, 2022 16:54
@DifferentialOrange
Copy link
Member Author

DifferentialOrange commented Feb 1, 2022 •
edited
Loading

Well, CI job failed, so it looks like I need to research so no flaky test will be introduced
logs_115.zip

After I increased timeouts a bit, there was 20 consecutive CI runs without fails, so I think I won't waste any more time.

LeonidVas
LeonidVas approved these changes Feb 1, 2022
View reviewed changes
multi/multi_test.go Outdated Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
test_helpers/main.go Outdated Show resolved Hide resolved
test_helpers/main.go Show resolved Hide resolved
uuid/uuid_test.go Show resolved Hide resolved
uuid/uuid_test.go Outdated Show resolved Hide resolved
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch 3 times, most recently from 922321c to 491e9aa Compare February 3, 2022 16:17
vr009
vr009 approved these changes Feb 7, 2022
View reviewed changes
Copy link

@vr009 vr009 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

ligurio
ligurio requested changes Feb 7, 2022
View reviewed changes
deps.sh Outdated Show resolved Hide resolved
.luacheckrc Outdated Show resolved Hide resolved
@ligurio ligurio force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from 71e736d to aa80991 Compare February 7, 2022 09:22
ligurio
ligurio requested changes Feb 7, 2022
View reviewed changes
deps.sh Outdated Show resolved Hide resolved
uuid/uuid_test.go Show resolved Hide resolved
uuid/uuid_test.go Show resolved Hide resolved
@DifferentialOrange
code: run go fmt
a3748a0
@DifferentialOrange
test: handle everything with go test
c847e79 
Before this patch, it was required to set up test tarantool processes
manually (and also handle their dependencies, like making working dir).
You can see an example in CI scripts.

This patch introduces go helpers for starting a tarantool process and
validating Tarantool version. Helpers are based on `os/exec` calls.
Retries to connect test tarantool instance handled explicitly,
see #136.

Setup scripts are reworked to use environment variables to configure
`box.cfg`. Listen port is set in the end of script so it is possible
to connect only if every other thing was set up already.

Every test is reworked to start a tarantool process (or processes) in
TestMain before test run.

To run tests, install all dependencies with running `make deps`
and then run `make test`. Flag `-p 1` in `go test` command means no
parallel runs. If you run tests without this flag, several test
tarantool instances will try to bind the same port, resulting in run
fail.

Closes #107
@DifferentialOrange
multi: describe init script logic
8f1cef6
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from aa80991 to bd1263f Compare February 8, 2022 11:44
@DifferentialOrange
lua: fix code style in test scripts
861a33f 
Added standard .luacheckrc config. Added indentation to box.once blocks.
Fix trailing spaces and add missing spaces to mathematical expressions.
Set global variables through rawset.
@DifferentialOrange DifferentialOrange force-pushed the DifferentialOrange/gh-107-self-sufficing-tests branch from bd1263f to 861a33f Compare February 14, 2022 16:26
@DifferentialOrange DifferentialOrange merged commit 17b725f into master Feb 14, 2022
@DifferentialOrange DifferentialOrange deleted the DifferentialOrange/gh-107-self-sufficing-tests branch February 14, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ligurio ligurio ligurio requested changes

@vr009 vr009 vr009 approved these changes

@LeonidVas LeonidVas LeonidVas approved these changes

@funny-falcon funny-falcon Awaiting requested review from funny-falcon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make test run self-sufficing
4 participants
@DifferentialOrange @ligurio @vr009 @LeonidVas