summaryrefslogtreecommitdiff
path: root/HACKING.rst
Commit message (Collapse)AuthorAgeFilesLines
* Add a hacking rule for the setDaemon methodTakashi Natsume2022-11-141-0/+1
| | | | | | | | | | | Add the following hacking rule. * N372: Don't use the setDaemon method. Use the daemon attribute instead. Change-Id: Idb45421205f76d2d3b0576bd0504d261ed249edd Related-Bug: 1987191 Signed-off-by: Takashi Natsume <takanattie@gmail.com>
* Add missing descriptions in HACKING.rstTakashi Natsume2022-08-311-0/+5
| | | | | | | | | | | This patch is a follow-up patch for the following changes. Ia7bcb40a21a804c7bc6b74f501d95ce2a88b09b5 Ie107a95bc06390ab519d3b3af9b07103a9a14316 I71439580e80d33cff62aba807df2b35164a47cbe Change-Id: I244396fb96b01ebab875234f25097637ff1b1dbb Signed-off-by: Takashi Natsume <takanattie@gmail.com>
* Add two new hacking rulesBalazs Gibizer2021-09-011-0/+3
| | | | | | | | | | | | | | | | | | | | | | As the bug and fix If71620e808744736cb4fe3abda76d81a6335311b showed it is dangerous to forget instantiating the Mock class before it is used in the test as changes on the class directly leaks out from the test. In almost all the cases using Mock class directly is a bug and the author original intention is to use an instance instead, just forgot about the parents. So this patch adds two new hacking rules: N367: catches the case when Mock class is aliased in the test: self.mock_mystuff = mock.Mock N368: catches when mock.patch instructed to use the Mock class as replacement value during patching: mock.patch('Bar.foo', new=mock.Mock) For N367 the previous patch removed the last hit. For N368 this patch removes the two hits exists. Change-Id: Id42ca571b1569886ef47aa350369e9d2068e77bc Related-Bug: #1936849
* Add a hacking rule for assert_has_callsTakashi Natsume2020-09-281-0/+1
| | | | | | | | | | | | | | | Add the following hacking rule. * N366: The assert_has_calls is a method rather than a variable. Not correct: mock_method.assert_has_calls = [mock.call(0)] Correct: mock_method.assert_has_calls([mock.call(0)]) This patch is a follow-up patch for Id094dd90efde09b9a835d4492f4a92b8f8ad296e. Change-Id: I892f8c23ee44f2b3518776a9705e3543f3115cae Signed-off-by: Takashi Natsume <takanattie@gmail.com>
* Remove hacking rules for python 2/3 compatibilityTakashi Natsume2020-06-171-5/+0
| | | | | | | | | | | | | | | | | | | The Python 2.7 Support has been dropped since Ussuri. So remove hacking rules for compatibility between python 2 and 3. - [N325] str() and unicode() cannot be used on an exception. Remove or use six.text_type() - [N327] Do not use xrange(). xrange() is not compatible with Python 3. Use range() or six.moves.range() instead. - [N344] Python 3: do not use dict.iteritems. - [N345] Python 3: do not use dict.iterkeys. - [N346] Python 3: do not use dict.itervalues. See also line 414 in https://etherpad.opendev.org/p/nova-victoria-ptg Change-Id: If4335b2e8ef5bbabba37598110c1aa8269635c2f Implements: blueprint six-removal Signed-off-by: Takashi Natsume <takanattie@gmail.com>
* hacking: Modify checks for translated logsStephen Finucane2020-05-271-1/+1
| | | | | | | | | | | | | | | | The N319 check previously asserted that debug-level logs were not translated. Now that we've removed all log translations, we can generalize this to all logs. We reuse the same number since these numbers are really just metadata and not public contracts. This also allows us to update the N323 and N326 checks, which ensure we import the translation function, '_', wherever it's used and don't concatenate translated and non-translated strings. Since we're no longer translating logs and the '_LE', '_LW' and '_LI' symbols are no longer provided, we don't need to consider logs in either of these cases. Change-Id: I64d139ad660bc382e8b9d7c8cd03352b26aadafd Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
* Merge "Make it easier to run a selection of tests relevant to ongoing work"Zuul2019-11-221-0/+24
|\
| * Make it easier to run a selection of tests relevant to ongoing workAdam Spiers2019-08-191-0/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | During development of a new git commit, locally running a whole unit or functional test suite to check every minor code change is prohibitively expensive. For maximum developer productivity and happiness, it's generally desirable to make the feedback loop of the traditional red/green cycle as quick as possible. So add run-tests-for-diff.sh and run-tests.py to the tools/ subdirectory, using a few tricks as explained below to help with this. run-tests.py takes a list of files on STDIN, filters the list for tests which can be run in the current tox virtualenv, and then runs them with the correct stestr options. run-tests-for-diff.sh is a simple wrapper around run-tests.py which determines which tests to run using output from "git diff". This allows running only the test files changed/added in the working tree: tools/run-tests-for-diff.sh or by a single commit: tools/run-tests-for-diff.sh mybranch^! or a range of commits, e.g. a branch containing a whole patch series for a blueprint: tools/run-tests-for-diff.sh gerrit/master..bp/my-blueprint It supports the same "-HEAD" invocation syntax as flake8wrap.sh (as used by the "fast8" tox environment): tools/run-tests-for-diff.sh -HEAD run-tests.py uses two tricks to make test runs as quick as possible: 1. It's (already) possible to speed up running of tests by source'ing the "activate" file for the desired tox virtualenv, e.g. source .tox/py36/bin/activate and then running stestr directly. This saves a few seconds by skipping the overhead introduced by running tox. 2. When only one test file needs to be run, specifying the -n option to stestr will skip the costly test discovery phase, saving several more valuable seconds. Future commits could build on top of this work, harnessing a framework such as watchdog / watchmedo[0] or Guard[1] in order to automatically run relevant tests every time your editor saves changes to a .py file. [0] https://github.com/gorakhargosh/watchdog - Python-based [1] https://guardgem.org - probably best in class, but Ruby-based so maybe unacceptable for use within Nova. Change-Id: I9a9bda5d29bbb8d8d77f769cd1abf7c42a18c36b
* | Remove descriptions of nonexistent hacking rulesTakashi NATSUME2019-08-261-5/+1
| | | | | | | | | | | | | | | | | | | | | | | | N321, N328, N329, N330 hacking rules have been removed since I9c334162fe1799e7b24563fdc11256b91bbafc9f. However the descriptions are still in HACKING.rst. So remove them. The rule number N307 is missing in HACKING.rst. So add it. Change-Id: I868c421a0f5a3329ab36f786f8519accae623f1a Closes-Bug: #1841400
* | Add a hacking rule for useless assertionsTakashi NATSUME2019-08-211-0/+1
| | | | | | | | | | | | | | | | Add a hacking rule for useless assertions in tests. [N365] Misuse of assertTrue/assertIsNone Change-Id: I3f76d57d75a266eddf7a4100c0b39fabe346e71c
* | Add a hacking rule for non-existent assertionsTakashi NATSUME2019-08-211-0/+1
| | | | | | | | | | | | | | | | | | | | | | Add a hacking rule for non-existent mock assertion methods and attributes. [N364] Non existent mock assertion method or attribute (<name>) is used. Check a typo or whether the assertion method should begin with 'assert_'. Change-Id: Ic6860e373120086a1a2ae9953f09a7bbaa032a7b
* | Fix missing rule description in HACKING.rstTakashi NATSUME2019-08-211-0/+1
|/ | | | | | | | | The description of N363 hacking rule is missing in HACKING.rst. Add the description. Change-Id: I036a48612fcd256df4ccbd2ebba814bf3ed7a1c2 Closes-Bug: #1840862
* Hacking N362: Don't abbrev/alias privsep importMichael Still2019-04-041-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As noted in [1]: We always import privsep modules like this: import nova.privsep.libvirt Not like this: from nova.privsep import libvirt This is because it makes it obvious at the caller that a priviledged operation is occuring: nova.privsep.libvirt.destroy_root_filesystem() Not just: libvirt.destroy_root_filesystem() This is especially true when the imported module is called "libvirt", which is a very common term in the codebase and super hard to grep for specific uses of. This commit introduces hacking rule N362 to enforce the above. Change-Id: I9b6aefa015acbf28e49a9ff1713a8bb544586579 Co-Authored-By: Eric Fried <openstack@fried.cc>
* Document how to make tests log at DEBUG levelAdam Spiers2019-02-131-0/+4
| | | | | | | OS_DEBUG is a handy trick, so let's make it slightly more discoverable. Change-Id: I02e4fbffc3c2c4f5b2737a44581ba7168f3a0d0f
* Add a hacking rule for deprecated assertion methodsTakashi NATSUME2018-10-251-0/+2
| | | | | | | | | | | | | | | Add a hacking rule for the following deprecated methods(*) in Python 3. * assertRegexpMatches * assertNotRegexpMatches [N361] assertRegex/assertNotRegex must be used instead of assertRegexpMatches/assertNotRegexpMatches. *: https://docs.python.org/3.6/library/unittest.html#deprecated-aliases Change-Id: Icfbaf26a7db6986820e264d1888982b985d613a1
* Removed unnecessary parantheses in yield statementsTakashi NATSUME2018-03-071-0/+1
| | | | | | | The 'yield' statement is not a function. So it must always be followed by a space when yielding a value. Change-Id: Ie2aa1c4822d8adf721ba71d467b63209bede6fb7
* Add check for redundant import aliasesesberglu2018-02-261-0/+1
| | | | | | | | | | | This adds a pep8 function that will check for redundant import aliases. Any imports of the forms below will not be allowed. from x import y as y import x as x import x.y as y Change-Id: Iff90f0172d97bd1d49d54c811a70c8af11776da4
* fix linkmelissaml2018-02-071-1/+1
| | | | Change-Id: I2695e5d848877dfbb13bacb04b042ad7ac9d1aad
* doc: fix link to creating unit tests in contributor guideMatt Riedemann2017-11-141-10/+12
| | | | | | | | | | | | | | | | | | | | | The testing strategy doc was linking to the hacking repo docs on creating unit tests, which are very specific to creating unit tests for hacking rules. This changes the link to the 'creating unit tests' section in the HACKING.rst file, which has more information on testing within nova. Along with that change, the HACKING.rst testing section is updated a bit to point out that we use stestr now instead of testr and adds a proper link to the development environment quickstart docs. The nova/tests/unit/README.rst actually needs a lot of work, but that's left for another day. Change-Id: Ie5106d87d632286162b31ce132e947c306d21abd Closes-Bug: #1732024
* Enable global hacking checks and removed local checksGábor Antal2017-02-101-4/+0
| | | | | | | | | | | | | | | | | | | | In this patchset, I set up 'enabled-extensions' in flake8 section of tox.ini. In newer hacking versions, we can enable some of the non-default hacking rules (one by one), which are disabled by default and implemented in the newer versions of hacking. The enabled extensions are the following: * [H106] Don’t put vim configuration in source files (off by default). * [H203] Use assertIs(Not)None to check for None (off by default). * [H904] Delay string interpolations at logging calls (off by default). Together with enabling these rules, I also removed the locally implemented versions of them. Due to limitations of local checking engine, there were some places in the tests, where pep8 failed. In this patchset, these codes are also fixed. Change-Id: I3a9d2dc007a269cdb2cad441e22f5eb9b58ce0a0
* Removed unnecessary parantheses and fixed formationGábor Antal2017-02-091-0/+1
| | | | | | | | | | | | | | | | | | | | | | | Some place in Nova, return value has unnecessary parentheses, or return keyword is not followed by a space when returning a value, which is a bad format and might be ambigous. For example: return(1) return{ 'a': 1 } As noone would write returns like: return1 return'(empty)' In this patchset we get rid of this bad formation. Also, I've added a hacking rule which helps to prevent this in future. TrivialFix Change-Id: I4ff85d0240bf8719188ebd06fc0e98a81c1d1203
* hacking: Use uuidutils or uuidsentinel to generate UUIDhussainchachuliya2016-11-291-0/+2
| | | | | | | | Added hacking check to ensure that UUID is being generated from oslo_utils.uuidutils or uuidsentinel(in case of test cases) instead of uuid4(). Change-Id: I73ee63fbd4f451d3aa5dc1a2a734d68c308b4440
* hacking: Use assertIs(Not), assert(True|False)Stephen Finucane2016-10-121-0/+2
| | | | | | | This is per the OpenStack style guidelines. Change-Id: Iec102872e2d5b004255ce897cc22c4d1a41c6f9e Co-authored-by: Gabor Antal <antal@inf.u-szeged.hu>
* Add a hacking rule for string interpolation at loggingTakashi NATSUME2016-10-111-0/+1
| | | | | | | | | | | | | | | | String interpolation should be delayed to be handled by the logging code, rather than being done at the point of the logging call. So add the following hacking rule for it. - [N354] String interpolation should be delayed at logging calls. See the oslo i18n guideline. * http://docs.openstack.org/developer/oslo.i18n/guidelines.html Change-Id: Ief6d3ee3539c0857098fffdb7acfeec3e0fed6eb Closes-Bug: #1596829
* Remove context object in oslo.log methodSivasathurappan Radhakrishnan2016-09-271-0/+1
| | | | | | | | | | Removed context object while logging as Nova uses oslo.context's RequestContext which means the context object is in scope when doing logging. Added hack to notify, in case if someone uses it in logging statements in future. Change-Id: I5aaa869f2e6155964827e659d18e2bcaad9d866b Closes-Bug:#1500896
* hacking: Always use 'assertIs(Not)None'Stephen Finucane2016-09-231-2/+2
| | | | | | This is per the OpenStack style guidelines. Change-Id: Ia706045fe3524b6b5e1db0140672776d481a0c01
* Add hacking checks for xrange()sonu.kumar2016-09-221-0/+1
| | | | | | Partially-Implements: blueprint goal-python35 Change-Id: Iea2e9e5d5782b898ba5b18314745962cc059a213
* Remove hacking check [N347] for config options.Markus Zoeller2016-08-111-1/+0
| | | | | | | The check for help texts (N347) can be better done by humans. This change removes it. Change-Id: I0861c94f764d559be8f188d3e4b54b2b8ad39ea5
* Replace deprecated LOG.warn with LOG.warningyatin karel2016-07-201-0/+1
| | | | | | | | LOG.warn is deprecated. It is still used in few modules. Replaced with non-deprecated LOG.warning. Change-Id: Ia6acc11eca60c652844175a5742f626732e295e3 Closes-Bug: #1508442
* Hacking check for _ENFORCER.enforce()Andrew Laski2016-07-011-0/+1
| | | | | | | | | | In order to ensure that only registered policies are used for authorization checks _ENFORCER.authorize should be used rather than _ENFORCER.enforce. This adds a check to look for instances of _ENFORCER.enforce being used. Change-Id: Iee78e93a3e1d4c6c30681b18698b7fc9cb6fa982 Implements: bp policy-in-code
* Hacking check for policy registrationAndrew Laski2016-07-011-0/+1
| | | | | | | | | Ensure that policy registration happens in the centralized nova/policies/ directory. There is an exception for the test_policy unit tests because some of them register rules for testing. Change-Id: Ia51eeb09eff86f82528b27bdc11a71762dfaed1a Partially-Implements: bp policy-in-code
* Merge "Add a hacking check for test method closures"Jenkins2016-04-041-0/+1
|\
| * Add a hacking check for test method closuresDan Smith2016-03-171-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A recurring pattern when using multiple mocks is to create a closure decorated with mocks like: def test_thing(self): @mock.patch.object(self.compute, 'foo') @mock.patch.object(self.compute, 'bar') def _do_test(mock_bar, mock_foo): # Test things _do_test() However it is easy to leave off the _do_test() and have the test pass because nothing runs. This check looks for that pattern. Co-Authored-By: Andrew Laski <andrew@lascii.com> Change-Id: I4c2395a01470acc7c9e5bcf1d3578d00270a2c07
* | Removes some redundant wordsAnh Tran2016-03-251-2/+2
|/ | | | | | This patch removes some redundant words. Change-Id: I1461ad1d98272b0d6223fd989861885902c12617
* Hacking: check for deprecated os.popen()kairoaraujo2016-02-191-0/+1
| | | | | | | | | Add hacking check for deprecated library function os.popen(). This hacking prevents new os.popen() in the code. Closes-Bug: 1529836 Change-Id: I09ad101861b790d2f9bec45757b8c921e68b696e
* config options: add hacking check for help text lengthMarkus Zoeller2016-02-031-0/+1
| | | | | | | | | | Adds a hacking check if a config option provides enough help text. To do so, the check counts the length of the help text. It uses a low number in order to avoid a break. As soon as we have agreed on a higher standard and changed the options accordingly, we can increase that number. Change-Id: If31339c428953c6a7bf721ff92e5999e8cb5b569
* Python3: Replace dict.iteritems with six.iteritemsabhishekkekane2016-01-211-0/+3
| | | | | | | | | | | | | | | | | This also adds a check to nova/hacking/checks.py that should check dict.iteritems, dict.itervalues and dict.iterkeys should not be used in the future. NOTE: In _dict_from_object() method of test_db_api.py items() method is called on dict and iteritems() method is called if object is other than dict. Changed it to directly use obj.items as obj can either be dict or object of nova.db.sqlalchemy.models.<Class> and both have items() method. Partially-Implements: blueprint nova-python3-mitaka Change-Id: Ib0fe3066199b92e4f013bb15312ada7515fa3d7b
* hacking: check for common double word typosDaniel P. Berrange2016-01-141-0/+1
| | | | | | | Adds a local hacking check for common double word typos like "the the", "to to", etc. Change-Id: I824a4a4b0f3bc18a16d5df50bd66cc626888ebaf
* add hacking check for config options locationMarkus Zoeller2015-11-261-0/+1
| | | | | | | This adds a hackign check which raises a pep8 error if a config option was found in location where it shouldn't be. Change-Id: I75df3404cc6178aa179e96d75f154e389c3008af
* hacking check for contextlib.nested for py34 supportDavanum Srinivas2015-10-161-0/+1
| | | | | | | | | | Removed use of contextlib.nested call from codebase as contextlib.nested is not compatible with Python 3. Added hacking check to catch if any new instances are added to the codebase. Change-Id: Ib78102bc013e4cc91ba54d79aa2815f4cf9f446d
* Merge "Add hacking check for eventlet.spawn()"Jenkins2015-10-081-1/+1
|\
| * Add hacking check for eventlet.spawn()Ryan Rossiter2015-08-131-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Change Id52c30bb5ded2184d772e6026b0f04f9a0efeb55 added a hacking check for greenthread.spawn(). Since eventlet.spawn() calls greenthread.spawn() under the covers, it should also be checked. Because there are still occurrences of eventlet.spawn(), these were also cleaned up in order to pass the added hacking check. Co-Authored-By: Qin Zhao <chaochin@gmail.com> Change-Id: Ia125b4ad5e84bf48091af5a7a483b89629f0ec31 Related-Bug: #1404268 Closes-Bug: #1468513
* | Fix typo in HACKING.rstSimon Pasquier2015-09-161-1/+1
|/ | | | Change-Id: Ib4d0beb033211db63148d3de0a46e9f6e11904f9
* Add hacking check for greenthread.spawn()Ryan Rossiter2015-08-111-0/+1
| | | | | | | | | | | | Because greenthread.spawn() and spawn_n() are missing a nova context (see I3623e60c49e442e2431cf017540422aa59bc285a and Ia5175c9729069df3d779237ba6039cf5bc920018), nova.utils.spawn() and spawn_n() should be used when spawning threads. This change adds a hacking check to assert this is being done during pep8. Change-Id: Id52c30bb5ded2184d772e6026b0f04f9a0efeb55 Related-Bug: #1404268 Closes-Bug: #1468513
* Remove unnecessary oslo namespace import checksDavanum Srinivas2015-07-231-1/+0
| | | | | | | | | Latest oslo libraries do not support the old oslo namespace based imports, so the import check in our hacking rules are redundant as CI jobs will fail for sure if someone tries to use oslo namespace based imports. Change-Id: I49c74ade374582f53a3678a1bc7df194c24e892e
* Merge "Update HACKING.rst for running tests and building docs"Jenkins2015-07-201-3/+3
|\
| * Update HACKING.rst for running tests and building docsMatt Riedemann2015-07-141-3/+3
| | | | | | | | | | | | | | | | | | | | | | There are three updates here: 1. Point to the correct path of development.environment.rst. 2. Update the path to test_wsgi.py after the test restructure that happened in Kilo. 3. Just tell people to use `tox -e docs` for building docs. Change-Id: I03295a6d9c90e9a2962999726d254bc4971c4909
* | Add tool to build a doc latex pdfMatthew Treinish2015-07-141-9/+11
|/ | | | | | | | | | | | The sphinx latex generation generates invalid latex that won't compile when you try to use it.[1][2] This commit adds a helper script to generate the sphinx latex and then modify it so it'll work. It depends on ImageMagick convert and sed being available to work. [1] https://github.com/sphinx-doc/sphinx/issues/1907 [2] https://github.com/sphinx-doc/sphinx/issues/1959 Change-Id: Id289c10907aaddae2483f18b39063852ec699d66
* Add a hacking rule for consistent HTTP501 messageKen'ichi Ohmichi2015-06-081-0/+1
| | | | | | | | | | There is raise_feature_not_supported() for returning a HTTP501 response with consistent error message, and this patch adds a rule for enforcing to use the method on v2.1 API. Partially implements blueprint v2-on-v3-api Change-Id: I06f254fd9c8d8b6aac4ed135c6c407f3a993431f
* Merge "Add note on running single tests to HACKING.rst"Jenkins2015-03-031-0/+8
|\
View Lorry Controller Status