Remove unnecessary thread_ident assignment

[emlove]

Description:

I'm hoping this will help fix some of the test instability. get_test_home_assistant calls async_test_home_assistant, which is running the event loop normally, so the patching isn't necessary.

[mention-bot]

@armills, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @kellerza to be potential reviewers.

[balloob]

I don't think this fixes it. The idea is that you call the method inside the loop, so we get the identifier of the thread. Somehow pytest ends up re-using that thread for doing other things.

I've been keeping a list of ones that fail and wonder if we should just rewrite them as async tests…

[balloob]

So I recall that it tends to happen more frequent when we have tests that are written as standalone methods but are not async. Going to test if I can convert all those to async.

[balloob]

nevermind, we have too many for my last comment to be true.

[balloob]

If we look at the history, it seems to start 20 days ago with build 24930 which is PR #7841. The PR doesn't seem to show any bad coding practice.

[balloob]

We are not in the thread that is the loop here. We are in a fixture which is executed synchronously. That's why the previous line actually calls loop.run-until_complete

[balloob]

This is inside the async thread because it's run inside a coroutine. However, I do realize now that this might actually not be the case because during the fixture we're calling loop.run_until_complete, which means that we're actually in a thread that executes the fixture!

We need to somehow schedule this so that it gets executed on the loop.

[balloob]

I guess we should remove it regardless, as it's wrong.

[balloob]

We should consider running our threads with PYTHONASYNCIODEBUG=1. It will be slower but it will also raise when async methods called from a sync context.

The reason I added the thread check originally was that Home Assistant gets into a deadlock when you call run_callback_threadsafe(…).result() from inside the event loop.

[balloob]

Found 3 tests that failed under PYTHONASYNCIODEBUG=1. Fixed two: #8200

[emlove]

I think the real question which I wasn't able to figure out myself is this: When we execute a pytest style test that isn't in a test case class and is decorated as a coroutine, it seems that it is executing in a different event loop than generated by the fixture for hass.loop. I think we to figure out how we can get a reference to the event loop object calling the test, and use that for hass.

@asyncio.coroutine
def test_example(hass):
    yield from test_method()
    assert True

[emlove]

If nothing else we can run PYTHONASYNCIODEBUG every now and then to check the tests.

[balloob]

Yeah, I think under the hood something like this happens inside 1 thread:

if async_test:
    loop.run_until_complete(test)
else:
    test

if tear_down:
    tear_down()  # Problem is here, now loop thread and sync tear down thread are the same!

[emlove]

So now that #8198 is merged, I don't think we're actually depending on this functionality directly in the tests any more.

I'm also of the opinion that if we did need it, we should be running hass.async_start from the tests, since we might be depending on other behavior as well.

[balloob]

This one is still great to have because this is actually a standalone thread running the loop. And since the check does catch errors, it would be nice to still have.

[balloob]

I hope our CI hurdles will be over now! Thanks