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

Enable paralleltest linter rule #2342

Open
1 task done
ademidoff opened this issue Jul 3, 2023 · 4 comments
Open
1 task done

Enable paralleltest linter rule #2342

ademidoff opened this issue Jul 3, 2023 · 4 comments
Assignees

Comments

@ademidoff
Copy link
Member

ademidoff commented Jul 3, 2023

Description

We consider parallel test a great speed booster when our tests run in CI environment. However, this is not always possible due to a number of constraint we have.

Suggested solution

  • evaluate if we can afford to enable the rule
  • if yes, fix all linter error to make tests pass

Additional context

The rule can be enabled by simply removing this line. You can check similar PRs to see how we enabled some other rules (ref: #1541)

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ademidoff ademidoff changed the title Enable the paralleltest linter rule Enable paralleltest linter rule Jul 3, 2023
@artemgavrilov artemgavrilov added good first issue Good issue for newcomers hacktoberfest labels Jul 3, 2023
@vishwas-sharma2480
Copy link
Contributor

hi @BupycHuk i am seeing these linter warning

admin/cmd/pmm-admin/main_test.go:27:1: Function TestPackages missing the call to method parallel (paralleltest)
func TestPackages(t *testing.T) {
^
admin/cmd/pmm-admin/main_test.go:37:1: Function TestVersionPlain missing the call to method parallel (paralleltest)
func TestVersionPlain(t *testing.T) {
^
admin/cmd/pmm-admin/main_test.go:46:1: Function TestVersionJson missing the call to method parallel (paralleltest)
func TestVersionJson(t *testing.T) {
^
admin/commands/config_test.go:27:1: Function TestConfigCommandArgs missing the call to method parallel (paralleltest)
func TestConfigCommandArgs(t *testing.T) {

should I resolve them using nolint

@vishwas-sharma2480
Copy link
Contributor

Hi @JiriCtvrtka an @BupycHuk any update on above ?

@BupycHuk
Copy link
Member

Hi @vishwas-sharma2480 looks like these tests can run in parallel

@vishwas-sharma2480
Copy link
Contributor

hi @BupycHuk and @ademidoff I have doubt

ia m usung t.Parallel() to fix the linterror for paralleltest
like
func TestListFromMySqlParams(t *testing.T) {
t.Parallel()
type testParams struct {
Params *agentpb.StartActionRequest_PTMySQLSummaryParams
to soleve lint error
agent/runner/actions/pt_mysql_summary_action_test.go:58:1: Function TestListFromMySqlParams missing the call to method parallel (paralleltest)
func TestListFromMySqlParams(t *testing.T) {

after I applied the changes I am getting this lint error

agent/runner/actions/pt_mysql_summary_action_test.go:58:6: TestListFromMySqlParams's subtests should call t.Parallel (tparallel)
func TestListFromMySqlParams(t *testing.T) {

can you please guide me where I am doing it wrong ?
Best Regards
@vishwas-sharma2480

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants