Commit Graph

124 Commits

Author SHA1 Message Date
Matt Riedemann f72b7e35c8 Update HACKING.rst for running tests and building docs
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
2015-07-14 11:56:45 -07:00
Ken'ichi Ohmichi 198a3bea23 Add a hacking rule for consistent HTTP501 message
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
2015-06-08 03:06:49 +00:00
Jenkins e9200ba51f Merge "Add note on running single tests to HACKING.rst" 2015-03-03 15:55:42 +00:00
Jenkins 36619550fb Merge "Treat LOG.warning and LOG.warn same" 2015-02-06 20:44:57 +00:00
Jenkins 9e304ebe26 Merge "Added hacking rule for assertEqual(a in b, True/False)." 2015-02-05 17:13:04 +00:00
jichenjc 4dddcf3643 Treat LOG.warning and LOG.warn same
8510d3aaee removed N331
checking, so now we need treat LOG.warn and LOG.warning
same, so this patch adds the same checking to LOG.warn.

Change-Id: Ifb3addfe116f8f7abb8750826b0217dfbd766439
2015-02-05 15:05:18 +08:00
jichenjc 8510d3aaee Remove N331 hacking rules
Per discussion in
https://review.openstack.org/#/c/145506/
https://review.openstack.org/#/c/135888/
there is no need to check whether warn or warning usage since
py3 support both. So according to the suggestion, this patch
removes N331 check.

Change-Id: I20a62c1d1e953f52a63565b8446f1561d178d5af
2015-01-27 06:10:19 +08:00
Kashyap Chamarthy cf472ab15c HACKING.rst: Update the location of unit tests' README.rst
As part of this commit 89cd6a0c49 ("move
all tests to nova/tests/unit") README.rst was moved from nova/tests/ to
nova/tests/unit/. Update HACKING.rst to reflect that.

Change-Id: I282baac560d6035e453542812c36b505ddc07bc1
2015-01-23 14:14:03 +01:00
Jenkins f7270765a1 Merge "Remove non existent rule N327 from HACKING.rst" 2015-01-22 21:07:35 +00:00
Sergey Nikitin 16774b6d33 Added hacking rule for assertEqual(a in b, True/False).
The following replacements were done in tests to have
clearer messages in case of failure:
- assertEqual(a in b, True) with assertIn(a, b)
- assertEqual(a in b, False) with assertNotIn(a, b)

The error message would now be like:
   'abc' not in ['a', 'b', 'c']
rather than:
   MismatchError: False != True

Change-Id: I514daca3a470feef5d332a1a319cba15256fc6ea
2015-01-22 11:57:44 +03:00
Mike Durnosvistov 6ab942dd19 Don't translate exceptions in tests
Exception lines in unit tests won't ever be run in production, no reason to
translate them.

Added hacking rule for not importing i18n translation in tests.

Change-Id: I92f546166d4e0b2fa8dc2018c6d3e268b8ec4c0b
2015-01-21 14:39:11 +02:00
ChangBo Guo(gcb) 69fef14509 Performance: leverage dict comprehension in PEP-0274
PEP-0274 introduced dict comprehensions to replace dict constructor
with a sequence of length-2 sequences, these are benefits copied
from [1]:
  The dictionary constructor approach has two distinct disadvantages
  from the proposed syntax though.  First, it isn't as legible as a
  dict comprehension.  Second, it forces the programmer to create an
  in-core list object first, which could be expensive.
Nova dropped python 2.6 support, we can leverage this now.
There is deep dive about PEP-0274[2] and basic tests about
performance[3].
Note: This commit doesn't handle dict constructor with kwagrs.
This commit also adds a hacking rule.

[1]http://legacy.python.org/dev/peps/pep-0274/
[2]http://doughellmann.com/2012/11/12/the-performance-impact-of-using-dict-instead-of-in-cpython-2-7-2.html
[3]http://paste.openstack.org/show/154798/

Change-Id: Ifb5cb05b9cc2b8758d5a8e34f7792470a73d7c40
2015-01-16 10:06:13 +08:00
Davanum Srinivas c3d10b7a62 Do not use deprecated assertRaisesRegexp()
The unit test log ends up with DeprecationWarning(s) from the outdated
calls to assertRaisesRegexp. We should switch to using assertRaisesRegex
instead. This commit eliminates these warnings from the log and the hacking
rule N344 ensures that folks don't end up adding fresh code down the line
with the outdated assertRaisesRegexp as well

Partial-Bug: #1407736

Change-Id: Ifba672f7568d5159c63bf88c534812e4e3a26d5a
2015-01-14 22:04:45 -05:00
Jenkins efe19fab90 Merge "Added hacking rule for assertTrue/False(A in B)" 2015-01-13 15:23:11 +00:00
Joe Gordon fe9072edde Remove non existent rule N327 from HACKING.rst
N327 was removed in e0d22ba4de

Change-Id: Ie433b4708d9df7fb3c6ac230c48dc953402b2055
2015-01-10 21:45:08 +00:00
Joe Gordon 571893ab42 Replace Hacking N315 with H105
Don't use author tags is H105 in hacking 0.10, so drop local rule N315
in favor of H105.

Change-Id: I38bf2ff5247817d58723f28e87607f16f3d9c374
2015-01-10 21:45:08 +00:00
Sergey Nikitin b6d30549c6 Added hacking rule for assertTrue/False(A in B)
The following replacements were done in tests to have
clearer messages in case of failure:
- assertTrue(a in b) with assertIn(a, b)
- assertTrue(a not in b) with assertNotIn(a, b)
- assertFalse(a in b) with assertNotIn(a, b)

The error message would now be like:
   'abc' not in ['a', 'b', 'c']
rather than:
   'False is not True'.

Change-Id: I92d039731f17b731d302c35fb619300078b8a638
2015-01-07 12:25:53 +03:00
Davanum Srinivas 19aa89149c Prevent new code from using namespaced oslo imports
Ib63da2d845843410634a1df0261af33b973daf32 is an example where
new code had to be fixed for oslo_concurrency imports. Let us
add a hacking check to prevent such regression. When we update
to new oslo libraries with non-namespace'd imports, we should
update the regexp in this hacking rule

Change-Id: I44536d477d06ddc1205b824bcb888b666405dce3
2014-12-28 16:00:47 +00:00
Matthew Gilliard f5c29e98c2 Adds hacking check for api_version decorator
Because of the implementation of this decorator and the controller's metaclass,
the api_version decorator must be the outermost (ie first) decorator wherever
it is used. This patch adds a hacking check to ensure that this is the case.

This decorator is intended to be used on multiple methods with the same name
which offends F811, so '#noqa' is needed too. This will be fixed in a separate
patch.

Bad:
    @some_decorator # noqa  <-- '# noqa' to avoid F811
    @api_version("2.5")     <-- Error, needs to be the first decorator
    def my_api_method...

Good:
    @api_version("2.5") # noqa   <-- this line still needs '# noqa' for F811
    @some_decorator
    def my_api_method...

Change-Id: I579c0061f03d788c477c5424d4d00ec7a6e721e1
2014-12-08 08:35:38 +00:00
Mike Durnosvistov e8c0b822f0 Replacement _ on _LW in all LOG.warning part 1
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating. For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack

There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages

Added a hacking rule for the warning about checking
translation for it and checking logging level `warning` instead
alias `warn`.

Change-Id: I2bced49dc5a0408a94d5d20d85b20c682886edbe
2014-11-20 11:19:16 +02:00
Mike Durnosvistov b7535793af Replacement _ on _LE in all LOG.exception
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating.  For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack

There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages

Added a hacking rule for the log exception about checking
translation for it.

Change-Id: If80ea6f177bb65afcdffce71550bb38fedcc54eb
2014-11-20 11:19:09 +02:00
Mike Durnosvistov 8431670ef8 Replacement _ on _LI in all LOG.info - part 1
oslo.i18n uses different marker functions to separate the
translatable messages into different catalogs, which the translation
teams can prioritize translating. For details, please refer to:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack

There were not marker fuctions some places in directory network.
This commit makes changes:
* Add missing marker functions
* Use ',' instead of '%' while adding variables to log messages

Added a hacking rule for the log info about checking
translation for it.

Change-Id: I96766d723b01082339876ed94bbaa77783322b8c
2014-11-20 10:19:17 +02:00
Ian Wienand b971870d64 Add note on running single tests to HACKING.rst
I haven't done this in a while, and I went through a number of false
starts forgetting the "--", trying to specify "filename:TestClass"
etc. before I got it to work.  Hopefully this saves someone else some
time.

Change-Id: I94590009157cce8d42002089304c66c164bcd6ec
2014-10-31 20:14:59 +11:00
James Carey 6683905b53 Remove use of unicode on exceptions
Casting exceptions to unicode using unicode() can interfere
with the proper translation of the exception message.  This
is especially true when lazy translation is enabled, since it
causes the exception message to be translated immediately using
the default locale.

This is the same problem caused by using str on exceptions,
which was fixed by https://review.openstack.org/#/c/116054/

In addition to fixing these cases, this patch updates
hacking check N325, which checks for the use of str() on
exceptions, to also check for the use of unicode().

In order to make it clear which cases that cannot be caught
by the hacking check have been inspected, they have been
converted to using six.text_type() instead of unicode().

Closes-bug: #1380806
Change-Id: I87bb94fa76458e028beba28d092320057b53f70a
2014-10-22 18:05:48 +00:00
Davanum Srinivas 45553a6dda mock.assert_called_once() is not a valid method
mock.assert_called_once() is a no-op that tests nothing.  Instead
with mock.assert_called_once_with() should be used (or use
assertEqual(1, mock_obj.call_count) if you don't want to check
parameters).

Add a new HACKING rule for nova to prevent assert_called_once()
usage from creeping in.

Closes-Bug: #1365751

Change-Id: I1055093a1c31792b6411a3cd46c80b8bcaaf78c1
2014-09-16 16:38:49 -04:00
Jenkins 1460e5e133 Merge "Move to oslo.db" 2014-09-07 12:58:22 +00:00
Andrey Kurilin 1b83b2f110 Move to oslo.db
Replace common oslo code nova.openstack.common.db by usage
of oslo.db library and remove common code.

Replaced catching of raw sqlalchemy exceptions by catches
of oslo.db exceptions(such as DBError, DBDuplicateEntry, etc).

Co-Authored-By: Eugeniya Kudryashova  <ekudryashova@mirantis.com>

Closes-Bug: #1364986
Closes-Bug: #1353131
Closes-Bug: #1283987
Closes-Bug: #1274523
Change-Id: I0649539e071b2318ec85ed5d70259c949408e64b
2014-09-05 14:18:36 +02:00
James Carey 4ef6c1d4ea Remove concatenation with translated messages
Translated messages should not be expanded by concatenation.  Instead
the additional text should be replacement text or part of the translated
message to allow the translators as much information as possible when
doing the translations.

Also, when lazy translation is enabled, the object returned does not
support concatenation and raises an exception.  Thus this patch is
needed to support blueprint: i18n-enablement.

This patch includes a hacking check for concatenation with a translated
message.

Change-Id: I8b368d275faa14b4750735445321874ce1da37d5
Partially-Implements: blueprint i18n-enablement
2014-08-27 21:47:49 -05:00
James Carey 298b0328d2 Remove use of str on exceptions
An exception's message can be a translatable message.  If it is, the message
can contain unicode characters which will cause str to fail.

In cases where the message is explicity needed, the use of str is replaced
with the use of six.text_type.  When an exception is used as a replacement
string in a format string, the logger correctly handles it without the
need for str, so it is removed.

In addition to the case where a translatable message contains unicode,
enabling lazy translation in the oslo.i18n library causes translatable
messages to be replaced with an object that does not support str, this
causes all calls to str on a translatable message to fail.  Thus this patch
is also needed to support blueprint: i18n-enablement.

This patch includes a hacking check for use of str() on exceptions identified
in except statements.

Change-Id: Idb426d7f710ea69b835f70d0e883e93e9b9111d2
Partially-Implements: blueprint i18n-enablement
2014-08-27 13:55:34 +01:00
Gary Kotton a0a6017f9b Hacking: a new hacking check was added that used an existing number
Commit 243879f5c5 added in a new hacking
check that used an existing number.

The new number is 324 (and not 323)

Change-Id: I7e604a408387438105c435ad16a1fa3d6491b642
Closes-bug: #1356815
2014-08-14 03:40:09 -07:00
Jay S. Bryant 416283bd35 Add hacking check for explicit import of _()
To ensure the right message catalog is used when translating
messages we need to make sure to explicitly import '_' in
any files that use that function.  We cannot count on
unit test to catch cases where the user has forgotten to
import the _() function.

This hacking check ensures that the function has been imported
anywhere that it is used.  Unit tests for the hacking check are
included.

Change-Id: I9d8101916bcb449345d3123617c2ac75776d053e
2014-08-03 11:57:08 -05:00
ChangBo Guo(gcb) 0bea84ac20 Removes the use of mutables as default args
Passing mutable objects as default args is a known Python pitfall.
We'd better avoid this. This commit changes mutable default args with
None, then use 'arg = arg or {}', 'arg = arg or []'. For unit code which
doesn't use the args , just set with None. This commit also adds hacking
check.

Closes-Bug: #1327473
Change-Id: I5a8492bf8ffef8e000b13b6bdfaef1968b96f816
2014-06-18 12:59:02 +08:00
Gary Kotton b4ef81d6a0 Add missing translation support
Update a number of files to add missing translation support.

The patch adds a new hacking check - N321. This ensures that
all log messages, except debug ones, have translations.

A '# noqa' indicates that the validation will not be done on
the specific log message. This should be used in cases where
the translations do not need to be done, for example, the log
message is logging raw data.

Closes-bug: #1290261
Change-Id: Ib2ca0bfaaf432e15448c96619682c2cfd073fbbc
2014-06-02 22:18:05 -07:00
Gary Kotton 5d7ac6a928 Update HACKING.rst to include N320
Commit 9235ada8c2 did not add in
the updated hacking rule.

Change-Id: If2daf6d2e7008c36d0aba6270ac034522dcb2e2b
Closes-bug: #1325812
2014-06-02 22:12:56 -07:00
Michael H Wilson dbc2b46e6e Add a reference to the nova developer documentation
I've showed these docs to more people than I care to. It would be
extremely helpful to reference them in HACKING, which is generally
the first thing people read if they intend to hack.

Change-Id: I9194d98f5525e29711b4a1b26414a50b8ceba525
2014-05-19 13:23:06 -06:00
Gary Kotton 8b894c3565 Hacking: add rule number to HACKING.rst
Commit ac8bce63f8 added hacking rules
N319. This was not documented in the file HACKING.rst.

Change-Id: Ibf7917aaada88db5afe1387859a387481ec05118
Closes-bug: #1313322
2014-04-27 05:03:33 -07:00
zhang-jinnan 33a459c2c3 Replace assertEqual(None, *) with assertIsNone in tests
Replace assertEqual(None, *) with assertIsNone in tests to have
more clear messages in case of failure.

Change-Id: I0d38a82e78fbe94657ab9a71c08422007843d179
2014-02-16 17:31:28 +08:00
Marcos Lobo d5e8f1af67 Change assertTrue(isinstance()) by optimal assert
Some of tests use different method of assertTrue(isinstance(A, B)) or
assertEqual(type(A), B). The correct way is to use assertIsInstance(A,
B) provided by testtools.

Change-Id: I4a5413f9d90d2e581044885a440a46bf3d76598f
Closes-Bug: #1268480
2014-02-12 16:19:08 +01:00
Michael Still 58f5579630 Remove @author from copyright statements.
We have git to track authorship, so let's not pad source files
with it as well.

Co-authored-by: Joe Gordon <joe.gordon0@gmail.com>

Change-Id: Ic2d62d6743f7716c086749cd99922b6c496771d4
2014-02-12 05:17:10 +11:00
Daniel P. Berrange 21213ee91c Renumber some nova hacking checks
Most of the nova hacking checks had picked numbers starting
from approx N300 onwards. Two recent additions randomly
picked N123 and N500. Renumber these to N313 and N314 and
make sure they're documented in HACKING.rst. Mention the
guidelines in the hacking source file for benefit of
future authors

Change-Id: Ia3eb4cb9a4ac7409db7eba9e1689f4a5780b8795
2014-02-10 16:42:13 +00:00
Daniel P. Berrange 848ea5ff01 Add hacking test to block cross-virt driver code usage
The baremetal driver has been accessing some libvirt driver
private classes and config variables. Any code that is intended
to be shared across virt drivers should be in a common package
instead. Add a hacking file to enforce this for imports and
config options so that no further problems like this are made
in the future.

NB, this skips verification of the baremetal driver until
bugs 1261826 and 1261827 are fixed, or the baremetal driver is
removed, whichever comes first.

Change-Id: Ifbfe597795795a847830f9bd937dc459cd37d118
Closes-Bug: #1261842
2014-02-04 18:01:11 +00:00
liu-sheng 74f953a1d7 Remove vi modelines
We don't need to have the vi modelines in each source file,
it can be set in a user's vimrc if required.

Also a check is added to hacking to detect if they are re-added.

Change-Id: I347307a5145b2760c69085b6ca850d6a9137ffc6
Closes-Bug: #1229324
2014-02-03 14:19:44 +00:00
Keshava Bharadwaj 113b321eae Updates OpenStack Style Commandments link
The current link in the HACKING file is broken. This references the
correct location for contributors to view.

Change-Id: I3cd4841035400d09a3e3e3369bb3a2a8c4bdee30
2013-10-16 21:52:11 +05:30
Roman Podolyaka 2e83be17d1 Use timeutils.utcnow() throughout the code
timeutils.utcnow() should be used instead of direct calls to
datetime.datetime.utcnow() to make it easy to override its return
value in tests.

Add a hacking rule to prevent regressions.

Fixes bug 1200141.

Change-Id: I170dbd7c9093bd627e2a0d5984b7ad1bf105c8d5
2013-07-19 16:08:54 +03:00
Devananda van der Veen f2b4419b9f Add HACKING check for db session param
Add a HACKING check to enforce that public db/api and db/sqlalchemy/api
methods to not accept a 'session' parameter.

This check is initially disabled, since it is failing ~24 times right
now, but will be enabled once bp/db-session-cleanup is complete.

Change-Id: Ib89eea58555032dd142d4e21e62d66e2726f0d06
2013-07-02 03:11:24 -07:00
Joe Gordon 958336e494 Clean up and make HACKING.rst DRYer
Reference the OpenStack hacking guide in HACKING.rst and remove
duplicated entries. Add new section for nova specific rules.

Change-Id: I704a49a0675acf1870953c76eba4978a1c4490eb
2013-07-01 15:48:46 +02:00
Jenkins 53f62b33ca Merge "Add notes about how doc generation works." 2013-06-13 02:33:10 +00:00
Thomas Bechtold 3100abbd9d Replace openstack-common with oslo in HACKING.rst
Change-Id: I8c6426983ec374e903c64108b990e5d4550e3fcc
2013-06-07 11:13:51 +02:00
Monty Taylor c067e2dc13 Add notes about how doc generation works.
Change-Id: I562c743798aeae5e49bd2f96944c3cacd776a53d
2013-06-02 07:48:02 -04:00
Dirk Mueller bf68a9592d Improve Python 3.x compatibility
Mechanical translation of the deprecated
except x,y: construct with except x as y:
The latter works with any Python >= 2.6.
Add Hacking check.

Change-Id: I845829d97d379c1cd9b3a77e7e5786586f263b64
2013-06-01 09:57:31 +02:00