Correctly check for search backend validation in search settings form

Review Request #9824 — Created March 22, 2018 and submitted

brennie
Review Board
release-3.0.x
4668
efd02b6...
reviewboard

The search settings form was assuming that the search_backend_id field
would be available in clean, but the value is only present if the
backend passes validation. We now check for its existence instead of
assuming that it does exist and carry on with validation accordingly.

  • Ran unit tests.
  • Selecting "elasticsearch" on the search settings form without it
    installed no longer results in a 500.
  • Enabling Elasticsearch works.
  • Enabling Whoosh works.
Description From Last Updated

I also want to see test cases for a correct backend, and info in Testing Done that you've tested all ...

chipx86chipx86

E501 line too long (84 > 79 characters)

reviewbotreviewbot

This should be two tests: One for a backend that doesn't exist, another for the validate() check.

chipx86chipx86

This line is too long.

chipx86chipx86

Ordering is all over the place. Can you fix this?

chipx86chipx86

Rather than the reset() above, can you use this pattern: register try: ... finally: unregister

chipx86chipx86

The "plain" version of a test should always go above the other ones. This should be first.

chipx86chipx86

F841 local variable 'form' is assigned to but never used

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Ordering is still off here.

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

brennie
chipx86
  1. 
      
  2. reviewboard/admin/tests.py (Diff revision 2)
     
     
     
     

    This should be two tests: One for a backend that doesn't exist, another for the validate() check.

  3. reviewboard/admin/tests.py (Diff revision 2)
     
     

    This line is too long.

  4. 
      
chipx86
  1. 
      
  2. I also want to see test cases for a correct backend, and info in Testing Done that you've tested all backends in there.

  3. 
      
brennie
Review request changed

Testing Done:

~  

Ran unit tests.

~   Selecting "elasticsearch" on the search settings form without it
~   installed no longer results in a 500.

  ~
  • Ran unit tests.
  ~
  • Selecting "elasticsearch" on the search settings form without it
    installed no longer results in a 500.
  ~
  • Enabling Elasticsearch works.
  +
  • Enabling Whoosh works.

Commit:

-c956c13e78057ace5840ce8ea99ba60a6bfe72df
+306886b306b2e1029bd7120f11eea76d6d53a081

Diff:

Revision 3 (+64 -4)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
  1. 
      
  2. reviewboard/admin/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     
     

    Ordering is all over the place. Can you fix this?

  3. reviewboard/admin/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     

    Rather than the reset() above, can you use this pattern:

    register
    
    try:
        ...
    finally:
        unregister
    
  4. reviewboard/admin/tests.py (Diff revision 3)
     
     
     
     
     
     
     
     

    The "plain" version of a test should always go above the other ones. This should be first.

  5. 
      
brennie
Review request changed

Commit:

-306886b306b2e1029bd7120f11eea76d6d53a081
+ebfc44acef0532405496f53b1bab8aa3b2235f3e

Diff:

Revision 4 (+68 -5)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
chipx86
  1. 
      
  2. reviewboard/admin/tests.py (Diff revision 5)
     
     
     

    Ordering is still off here.

  3. 
      
brennie
david
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (ce0c109)
Loading...