Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Minor test improvements #1100

Merged
merged 5 commits into from
Mar 29, 2017
Merged

Minor test improvements #1100

merged 5 commits into from
Mar 29, 2017

Conversation

pdhamdhere
Copy link
Contributor

@pdhamdhere pdhamdhere commented Mar 28, 2017

  1. Add timestamps to logs
  2. Reduce volume size
  3. Reduce number of iterations for concurrency tests

Saves approximately 8-9minutes each CI run.

@pdhamdhere pdhamdhere changed the title [NOT FOR REVIEW] Test improvements Minor test improvements Mar 29, 2017
Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 left a comment

Choose a reason for hiding this comment

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

Proposed changes looks good to me. Minor comments to use t.Logf instead of fmt.printf().

@@ -250,6 +253,9 @@ func TestSanity(t *testing.T) {
volumeName, elem.endPoint)
}
}
fmt.Printf("%s END: Running TestSanity on %s (may take a while)...\n",
Copy link
Contributor

@shuklanirdesh82 shuklanirdesh82 Mar 29, 2017

Choose a reason for hiding this comment

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

minor: better to stick with other logging in this file using t.Logf() ... e.g. line 252

IIRC, you were suggesting to use log as it comes with in-built logging current time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True but I am trying to be consistent with START block at L213 'fmt.Printf("%s START:'

Copy link
Contributor

Choose a reason for hiding this comment

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

I see ... fine with me. We will take care during massive cleanup.

@shuklanirdesh82
Copy link
Contributor

Saves approximately 8-9minutes each CI run.

Great! Thanks for helping here!

@pdhamdhere
Copy link
Contributor Author

Thanks @shuklanirdesh82 for review. Since this is test-only minor change, merging with 1 approval.

@pdhamdhere pdhamdhere merged commit 7dc7014 into master Mar 29, 2017
@pdhamdhere
Copy link
Contributor Author

#1057

@pdhamdhere pdhamdhere deleted the wip branch March 29, 2017 05:08
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants