feat: convert dataframe tests

[betodealmeida]

SUMMARY

This converts the dataframe tests to unit tests. They're not pure unit tests, since some of the imports have side-effects that require mocking SQLALCHEMY_DATABASE_URI, but they run in a few seconds.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

Merging #17655 (f602672) into master (8e02d11) will increase coverage by 0.25%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17655      +/-   ##
==========================================
+ Coverage   68.66%   68.92%   +0.25%     
==========================================
  Files        1596     1597       +1     
  Lines       65224    65284      +60     
  Branches     6950     6950              
==========================================
+ Hits        44789    44999     +210     
+ Misses      18550    18400     -150     
  Partials     1885     1885              

Flag	Coverage Δ
hive	81.70% <ø> (?)
javascript	57.45% <ø> (ø)
mysql	82.11% <ø> (+0.30%)	⬆️
postgres	82.12% <ø> (+0.30%)	⬆️
presto	81.71% <ø> (+0.02%)	⬆️
python	82.61% <ø> (+0.53%)	⬆️
sqlite	81.81% <ø> (+0.30%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/key_value/commands/exceptions.py	100.00% <0.00%> (ø)
superset/dashboards/filter_state/commands/entry.py	100.00% <0.00%> (ø)
superset/utils/core.py	89.76% <0.00%> (+0.11%)	⬆️
superset/connectors/sqla/models.py	88.42% <0.00%> (+0.22%)	⬆️
superset/initialization/__init__.py	89.82% <0.00%> (+1.75%)	⬆️
superset/utils/feature_flag_manager.py	100.00% <0.00%> (+3.70%)	⬆️
superset/utils/logging_configurator.py	85.18% <0.00%> (+3.70%)	⬆️
superset/utils/cache_manager.py	100.00% <0.00%> (+3.84%)	⬆️
superset/db_engine_specs/hive.py	87.25% <0.00%> (+16.98%)	⬆️
superset/dashboards/filter_state/api.py	100.00% <0.00%> (+17.02%)	⬆️
... and 11 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e02d11...f602672. Read the comment docs.

[hughhhh]

any reason why these aren't at the top of the file?

[betodealmeida]

Yeah, these imports need to be done inside an app_context, otherwise they fail:

>>> from superset.db_engine_specs import BaseEngineSpec
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/beto/Projects/incubator-superset/superset/db_engine_specs/__init__.py", line 43, in <module>
    from superset.db_engine_specs.base import BaseEngineSpec
  File "/Users/beto/Projects/incubator-superset/superset/db_engine_specs/base.py", line 61, in <module>
    from superset.models.sql_lab import Query
  File "/Users/beto/Projects/incubator-superset/superset/models/__init__.py", line 17, in <module>
    from . import (
  File "/Users/beto/Projects/incubator-superset/superset/models/alerts.py", line 53, in <module>
    class Alert(Model, AuditMixinNullable):
  File "/Users/beto/Projects/incubator-superset/superset/models/alerts.py", line 66, in Alert
    owners = relationship(security_manager.user_model, secondary=alert_owner)
  File "/Users/beto/.pyenv/versions/superset/lib/python3.8/site-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
AttributeError: 'NoneType' object has no attribute 'user_model'
>>>

The app_context fixture will do the initialization needed for the import. This way, it works when ran inside the test, but fails as a top-level import.

Note that this is a consequence of bad architecture in Superset. You can see that when we import BaseEngineSpec it assumes that a user model is defined in the security manager, and it fails.

[hughhhh]

Suggested change

	def test_df_to_records(app_context: None) -> None:
	def test_df_to_records(_: None) -> None:

[betodealmeida]

We need app_context here to be named as is, since it's a fixture used by the test.

[hughhhh]

Suggested change

	def test_js_max_int(app_context: None) -> None:
	def test_js_max_int(_: None) -> None:

[betodealmeida]

Same here. It's a weird behavior of pytest, but we specify which fixtures we want based on their names.