[READY] Port to unittest

[bstaletic]

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your PR:

• I have read and understood YCM's CONTRIBUTING document.
• I have read and understood YCM's CODE_OF_CONDUCT document.
• I have included tests for the changes in my PR. If not, I have included a
  rationale for why I haven't.
• I understand my PR may be closed if it becomes obvious I didn't
  actually perform all of these steps.

Why this change is necessary and useful

Rationale is in ycm-core/ycmd#1591

[Please explain in detail why the changes in this PR are needed.]

---

This change is 

[codecov]

Codecov Report

Merging #3938 (482c769) into master (bb9ebb5) will decrease coverage by 0.13%.
The diff coverage is n/a.

❗ Current head 482c769 differs from pull request most recent head d75d806. Consider uploading reports for the commit d75d806 to get more accurate results

@@            Coverage Diff             @@
##           master    #3938      +/-   ##
==========================================
- Coverage   91.90%   91.76%   -0.14%     
==========================================
  Files          27       27              
  Lines        3668     3668              
==========================================
- Hits         3371     3366       -5     
- Misses        297      302       +5     

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

---

python/ycm/tests/omni_completer_test.py, line 740 at r1 (raw file):

  @YouCompleteMeInstance( { 'g:ycm_cache_omnifunc': 0, } )
  def test_OmniCompleter_GetCompletions_StartColumnCompliance( self, ycm ):

Notice that I have changed cache_omnifunc to 0. This is probably wrong.

Previously, with pytest, parametrized tests were treated as separate tests. That means ycm produced from @YouCompleteMeInstance( opts ) was unique to each test.
Now, with unittest, subtests are parts of a larger test. That means they all share the same ycm instance.

This has some odd consequences:

1. Each test passes on its own.
2. In a group, they fail, as some things get cached.
3. Perhaps a careful ordering of subtests would circumvent caching.

I don't think so, but we might have a bug. Either way, the hack to make this test pass should be reverted.

[puremourning]

Reviewed 24 of 24 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

---

python/ycm/tests/init.py, line 106 at r1 (raw file):

def YouCompleteMeInstance( custom_options = {} ):
  """Defines a decorator function for tests that passes a unique YouCompleteMe

remove

---

python/ycm/tests/init.py, line 129 at r1 (raw file):

        try:
          test_utils.VIM_MATCHES_FOR_WINDOW.clear()
          return test( args[ 0 ], ycm, *args[ 1: ], **kwargs )

test_case_instance

---

python/ycm/tests/omni_completer_test.py, line 740 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

Notice that I have changed cache_omnifunc to 0. This is probably wrong.

Previously, with pytest, parametrized tests were treated as separate tests. That means ycm produced from @YouCompleteMeInstance( opts ) was unique to each test.
Now, with unittest, subtests are parts of a larger test. That means they all share the same ycm instance.

This has some odd consequences:

1. Each test passes on its own.
2. In a group, they fail, as some things get cached.
3. Perhaps a careful ordering of subtests would circumvent caching.

I don't think so, but we might have a bug. Either way, the hack to make this test pass should be reverted.

thanks for the pointer, I would totally have missed this.

maybe the solution is to use YouCompleteMeInstance as a generator, e.g.

def test_Blah( self ):
  for stuff:
    with YouCompleteMeInstance() as ycm, self.subTest( .... ):
      # use ycm

not sure if the decorator can be used directly, or if we need to make another generator like with TempYCMInstance() or something

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

---

python/ycm/tests/init.py, line 106 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

remove

The whole comment, or?

---

python/ycm/tests/init.py, line 129 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

test_case_instance

Done.

---

python/ycm/tests/conftest.py, line 36 at r1 (raw file):

  # We treat warnings as errors in our tests because warnings raised inside Vim
  # will interrupt user workflow with a traceback and we don't want that.
  warnings.filterwarnings( 'error' )

This warnings thing got dropped and looks important.

Options:

1. Use -Wa on the command line.
2. Use PYTHONWARNINGS env var.
3. Implement this as setUpModule/tearDownModule and import everywhere.
4. unittest.main() provides access, but we're not using unittest.main(). We're running from the cli.

---

python/ycm/tests/omni_completer_test.py, line 740 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

thanks for the pointer, I would totally have missed this.

maybe the solution is to use YouCompleteMeInstance as a generator, e.g.

def test_Blah( self ):
  for stuff:
    with YouCompleteMeInstance() as ycm, self.subTest( .... ):
      # use ycm

not sure if the decorator can be used directly, or if we need to make another generator like with TempYCMInstance() or something

Decorators are only context managers if they have @contextlib.contextmanager on the function that yields values.

However, our @YouCompleteMeInstance doesn't yield ycm. It transforms the call to TestCase.test_whatever() and injects ycm as an argument.

I don't see a way to reconcile the two and we already have two versions of isolated ycmd (decorator and context) for ycmd/tests/java.

I did think of context managers, but I wanted to wait to see if you have a better idea.

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

---

python/ycm/tests/omni_completer_test.py, line 740 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

Decorators are only context managers if they have @contextlib.contextmanager on the function that yields values.

However, our @YouCompleteMeInstance doesn't yield ycm. It transforms the call to TestCase.test_whatever() and injects ycm as an argument.

I don't see a way to reconcile the two and we already have two versions of isolated ycmd (decorator and context) for ycmd/tests/java.

I did think of context managers, but I wanted to wait to see if you have a better idea.

The context manager worked.

[bstaletic]

Coverage is comlaining about s:TickSpinner() in finder.vim suddenly not being covered.

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

---

python/ycm/tests/conftest.py, line 36 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

This warnings thing got dropped and looks important.

Options:

1. Use -Wa on the command line.
2. Use PYTHONWARNINGS env var.
3. Implement this as setUpModule/tearDownModule and import everywhere.
4. unittest.main() provides access, but we're not using unittest.main(). We're running from the cli.

Opted for -We on the command line. Tested locally and it seemed to work.

[puremourning]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)

[puremourning]

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

---

python/ycm/tests/init.py, line 106 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

The whole comment, or?

Never mind it’s ok.

[puremourning]

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

[mergify]

Thanks for sending a PR!

[mergify]

Thanks for sending a PR!