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

Modify runtests.py to run a subset of tests based on filenames #47337

Merged
merged 54 commits into from Aug 15, 2018

Conversation

Projects
None yet
6 participants
@terminalmage
Copy link
Contributor

commented Apr 26, 2018

This adds two new options to runtests.py's option parser:

  • --from-filenames - A comma-separated list of filenames. runtests.py will then attempt to match test modules in both the unit and integration suites, by filename (e.g. salt/modules/git.py would result in unit.modules.test_git and integration.modules.test_git being run.
  • --filename-map - A supplemental YAML file which can be used to map file globs to test modules, allowing us to make sure that changes in a pkg module cause the pkg/pkgrepo state tests to be run (for example). Each glob will be fnmatch.filter'ed against the list of files, and if a match is found, the targeted list of test modules will be run.

Right now, the initial tests/filename_map.yml only contains one glob, I need to add more and would like some input on what should be added.

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Apr 27, 2018

This is cool. I really like this approach.

terminalmage added a commit to terminalmage/salt that referenced this pull request May 4, 2018

Rename pip state test modules to match naming convention
This makes the naming match the naming convention used elsewhere and
will make the changes in saltstack#47337
work better.

terminalmage added a commit to terminalmage/salt that referenced this pull request May 4, 2018

Rename pip state test modules to match naming convention
This makes the naming match the naming convention used throughout the
test suite and will make the changes in
saltstack#47337 work better.

@terminalmage terminalmage force-pushed the terminalmage:runtests-subset branch from 9a37240 to 8937e4c May 7, 2018

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented May 7, 2018

I've added some functionality to tests/support/parsers/__init__.py (details in this commit). I've also filled out the filename map with what I could identify with a couple passes through the state/execution modules.

@terminalmage terminalmage force-pushed the terminalmage:runtests-subset branch from 8937e4c to df02f10 May 10, 2018

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented May 10, 2018

I've improved the map file some more with some of the other unit test files. This should be ready for review now, and we should be able to start testing using this in develop once #47603 is merged forward to develop.

@terminalmage terminalmage changed the title WIP: Modify runtests.py to run a subset of tests based on filenames Modify runtests.py to run a subset of tests based on filenames May 10, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented May 15, 2018

@rallytime
Copy link
Contributor

left a comment

This is great! I just have one small question, plus the lint error should be fixed. Otherwise, really nice.


try:
# 2018.3 and later
from salt.utils.stringutils import expr_match as _expr_match

This comment has been minimized.

Copy link
@rallytime

rallytime May 15, 2018

Contributor

Why does this particular utils import need to be separated out in a try/except, but not the other imports above that have full salt.utils.x paths?

This comment has been minimized.

Copy link
@terminalmage

terminalmage May 15, 2018

Author Contributor

This is because the function location is different on 2017.7 due to the salt.utils re-organization. So the except below will try to import from the 2017.7-and-earlier location. The as is used so that irrespective of Salt version, we use the same name to invoke expr_match.

This comment has been minimized.

Copy link
@rallytime

rallytime May 15, 2018

Contributor

@terminalmage Right, but you have files above that are not in a try/except block that were also moved around during the salt.utils changes. My thought is that either all of those imports should be moved into a try/except, or none should.

This comment has been minimized.

Copy link
@terminalmage

terminalmage May 15, 2018

Author Contributor

Ahh yeah, I added those other imports later after initially doing it that way for expr_match. I removed the try/except. We'll either have to backport separately for 2017.7 and 2018.3 or not backport past 2018.3.


salt/modules/(mac_user|useradd|pw_user|solaris_user|win_useradd)\.py:
- unit.states.test_user
- integration.states.test_user

This comment has been minimized.

Copy link
@Ch3LL

Ch3LL May 22, 2018

Contributor

possibly add integration.modules.test_useradd and unit.modules.test_useradd?

and also on the other tests below it seems we are not clarifying the integration/unit.module tests equivalents as well. is this on purpose?

This comment has been minimized.

Copy link
@terminalmage

terminalmage Aug 14, 2018

Author Contributor

Yes, this is all on purpose. runtests.py will automagically match files that match a naming convention. Otherwise, the map file would be thousands of lines long and would be a pain in the butt to maintain.

So, there is no need to add integration.modules.test_useradd and unit.modules.test_useradd as they would be picked up when salt/modules/useradd.py is specified in the --from-filenames parameter.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2018

@terminalmage Bump - did you see the feedback from @Ch3LL?

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Aug 10, 2018

@terminalmage Was this the PR you were talking about earlier today?

@terminalmage terminalmage force-pushed the terminalmage:runtests-subset branch from f9534ee to fc59c6c Aug 13, 2018

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2018

@cachedout Perhaps?

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2018

This PR has been updated with the commits from #49091, as well as a unit test to enforce the naming convention. It still needs some local testing before it's ready for re-review, however.

@terminalmage terminalmage force-pushed the terminalmage:runtests-subset branch from 4bd16ef to 2e85639 Aug 14, 2018

@rallytime rallytime requested review from cachedout and gtmanfred Aug 14, 2018

terminalmage added some commits Aug 13, 2018

Rename providers to clouds to respect naming convention
Also, move rackspace openstack-based tests to test_openstack.py

@terminalmage terminalmage force-pushed the terminalmage:runtests-subset branch from 2e85639 to 20d5cd8 Aug 15, 2018

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2018

Rebased.

@dwoz

dwoz approved these changes Aug 15, 2018

@gtmanfred

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2018

@terminalmage there are a couple failed tests, they don't look like they would be related to me, but would you mind double checking?

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2018

@gtmanfred Already ahead of you. The pkg test is unrelated and I've opened #49134 to fix it. Looking at the sdb test now.

@gtmanfred gtmanfred merged commit 54a9984 into saltstack:develop Aug 15, 2018

4 of 9 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has failed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has failed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has failed
Details
jenkins/pr/py2-centos-7 running py2-centos-7...
Details
WIP ready for review
Details
codeclimate All good!
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2018

sdb tests are failing on the develop branch already and are also unrelated.

@terminalmage terminalmage deleted the terminalmage:runtests-subset branch Aug 15, 2018

terminalmage added a commit to terminalmage/salt that referenced this pull request Aug 20, 2018

Add the saltcli tests to the filename map salt/client/*
This updates the filename map added in saltstack#47337 to ensure that the salt
CLI tests are run when anything in salt.client files are updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.