snu-quiqcl / qiwis

QuIqcl Widget Integration Software
MIT License
5 stars 2 forks source link

Handle re-creating the existing app #207

Closed BECATRUE closed 11 months ago

BECATRUE commented 11 months ago

This PR will close #190.

I implement two things.

  1. Add the param replace to handle the case where trying to re-create an existing app.
  2. Update createApp() test and make subscribe() test

Suprisignly, we haven't tested for subscribe(), thus the coverage wasn't 100%. I tried to implement it, but encountered a minor problem.

I should handle the case where the app tries to subscribe a channel already subscribed to. But, I think there is no way to distinguish two cases (line 316 and 318), because we cannot mock set.add(). Can I test two cases with mocking logger?

BECATRUE commented 11 months ago

If you want to test both cases, you can mock the self.qiwis._subscribers dictionary.

It has a problem! At the beginning of subscribe(), it checks if app in self._subscribers[channel] so that it shouldn't be mocked. Another considerable solution is mocking self.qiwis._subscribers[channel].add function but it is blocked in Python.

kangz12345 commented 11 months ago

If you want to test both cases, you can mock the self.qiwis._subscribers dictionary.

It has a problem! At the beginning of subscribe(), it checks if app in self._subscribers[channel] so that it shouldn't be mocked.

Oh, I was talking about mock.patch.dict(self.qiwis._subscribers, {"ch": set()}) or something like that.

kangz12345 commented 11 months ago

If you want to test both cases, you can mock the self.qiwis._subscribers dictionary.

It has a problem! At the beginning of subscribe(), it checks if app in self._subscribers[channel] so that it shouldn't be mocked.

Oh, I was talking about mock.patch.dict(self.qiwis._subscribers, {"ch": set()}) or something like that.

I'm sorry now I get it. I will think about it.

kangz12345 commented 11 months ago

If you want to test both cases, you can mock the self.qiwis._subscribers dictionary.

It has a problem! At the beginning of subscribe(), it checks if app in self._subscribers[channel] so that it shouldn't be mocked. Another considerable solution is mocking self.qiwis._subscribers[channel].add function but it is blocked in Python.

How about exploiting the magic methods (note that MagicMock is Mock with magic methods implemented)? For example,

m = mock.MagicMock()
m.__contains__.side_effect = ("a", "b").__contains__

Then "a" in m returns True, "c" in m returns False, and all the mocked calls are recorded as well.

BECATRUE commented 11 months ago

Thanks for suggesting a solution😂

First, I solved the problem in my own way and please review it.

BECATRUE commented 11 months ago

Please check the updated parts! @kangz12345