fix(test): missing auth on tests #10842
Conversation
| result = json.loads(self.get_resp("/superset/results/key/")) | ||
| expected = {"status": "success", "query": {"rows": 100}, "data": data} | ||
| self.assertEqual(result, expected) | ||
| with mock.patch("superset.views.core.db") as mock_superset_db: |
There was a problem hiding this comment.
using the mock has a decorator was mocking for self.login() also
There was a problem hiding this comment.
Nice catch 👍 LGTM, but I wonder if we should standardize on how self.login is called. It appears the test codebase is mostly calling self.login("admin"), which is the exact same as self.login() due to defaults. I wonder if we should just make it mandatory to specify the username?
Yes, agree we should standardize, I'm planning on making of bunch of PR's to refactor tests to use pytest fixtures also, it may make sense to include that on those. |
* fix(test): missing auth on tests * fix mock * make test login more inline with other tests
mistercrunch
added
🏷️ bot
SUMMARY
Fixes some tests that were successful but were actually testing on a non authenticated user context (they were supposed to run with admin). This surfaced while cherry picking for 0.38.
ADDITIONAL INFORMATION