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

Execution speed improvement for functional tests #1899

Merged
merged 1 commit into from Apr 9, 2019
Merged

Execution speed improvement for functional tests #1899

merged 1 commit into from Apr 9, 2019

Conversation

themr0c
Copy link
Contributor

@themr0c themr0c commented Apr 2, 2019

Some rework on tests to enable performance and performance measurements.

  • improve measurements of performance with profiling callbacks
  • improve tests performance by disabling gather_facts
  • improve tests performance by replacing commend module by debug module

Signed-off-by: Fabrice Flore-Thebault themr0c@users.noreply.github.com

PR Type

  • Feature Pull Request

decentral1se
decentral1se approved these changes Apr 2, 2019
View reviewed changes
Copy link
Contributor

@decentral1se decentral1se left a comment •
edited

Choose a reason for hiding this comment

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

Note to our future selves (for another PR): let's make a molecule.yml in test/functional/scenarios which holds this default configuration (config_options: ...) and change molecule invocations to include --base-config ... so we do not have to repeat ourselves like this.

Looks great!

@decentral1se decentral1se added enhancement test Improvement to quality assurance: CI/CD, testing, building labels Apr 2, 2019
@decentral1se
Copy link
Contributor

Note to ourselves: let's make a molecule.yml in test/functional/scenarios which holds this default configuration (config_options: ...) and change molecule invocations to include --base-config ... so we do not have to repeat ourselves like this.

As an idea on how to do this, again, doesn't have to be this PR - we could add a 'default options' fixture somewhere around https://github.com/ansible/molecule/blob/master/test/functional/conftest.py#L46 and pass it into every test with the base_config options set to the correct path. We could then change the tests from options = { ... to options['...'] etc. This would help us set more general purpose options across the entire test suite.

@decentral1se decentral1se self-requested a review April 2, 2019 14:10
@decentral1se
Copy link
Contributor

Ah, I did introduce a bug in https://github.com/ansible/molecule/pull/1871/files#diff-8a0941ded1a1bf5411a8a1e5562a82d7R6 which is fixed here.

We're skipping all those tests, so I missed it 😅

@themr0c themr0c force-pushed the tests-improvements branch 3 times, most recently from 3603f9d to 6a232fb Compare April 9, 2019 13:10
@themr0c
Copy link
Contributor Author

themr0c commented Apr 9, 2019

  • Removed from PR some deeper changes that should not happen right now and did break the builds.
  • Squashed everything in a single commit with a nicer comment.
  • Didn't fix the codestyle errors, I guess it belongs to a dedicated PR.

@themr0c themr0c requested a review from gundalow April 9, 2019 13:15
@themr0c themr0c changed the title tests improvements Execution speed improvement for functional tests Apr 9, 2019
@themr0c themr0c force-pushed the tests-improvements branch 2 times, most recently from 3f1473d to 8ffeb0f Compare April 9, 2019 14:22
@themr0c
Execution speed improvement for functional tests.
04ccc78 
* add `callback_whitelist: profile_roles,profile_tasks,timer`,
* add `gather_facts: false` and don't use variables from setup,
* replace `command` task with `debug` task on host_group_vars scenario.

Signed-off-by: Fabrice Flore-Thebault <themr0c@users.noreply.github.com>
@themr0c
Copy link
Contributor Author

themr0c commented Apr 9, 2019

\o/ all checks passed, except the code style! ready to merge?

decentral1se
decentral1se approved these changes Apr 9, 2019
View reviewed changes
Copy link
Contributor

@decentral1se decentral1se left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this. Really useful!

@decentral1se decentral1se merged commit 4f07a1e into ansible:master Apr 9, 2019
@decentral1se decentral1se mentioned this pull request Apr 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@decentral1se decentral1se decentral1se approved these changes

@gundalow gundalow Awaiting requested review from gundalow

Assignees
No one assigned
Labels
enhancement test Improvement to quality assurance: CI/CD, testing, building
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@themr0c @decentral1se