Unfail anomaly detection tests#252
Conversation
| @@ -473,57 +467,95 @@ def test_RelativeRateOfChangeStrategy(self): | |||
| [Row(check_status="Success")], | |||
There was a problem hiding this comment.
There are now two methods named test_RelativeRateOfChangeStrategy in the same class. The first one (around line 408) tests various scenarios using the helper method self.RelativeRateOfChangeStrategy(...), and the second one (starting around line 476) is the newly uncommented test. Python will silently override the first with the second, so all the assertions from the first test_RelativeRateOfChangeStrategy (lines ~408-467) will never run. Rename one of them (e.g., test_RelativeRateOfChangeStrategy_detailed).
| # can not implement breeze.stats.DescriptiveStats, because it is not an interface | ||
| # (breeze.stats.DescriptiveStats is in unnamed module of loader 'app') | ||
| @pytest.mark.xfail(reason="TODO: breeze.stats.DescriptiveStats is in unnamed module of loader 'app'") | ||
| def test_BatchNormalStrategy(self): |
There was a problem hiding this comment.
The @pytest.mark.xfail decorator was removed from test_BatchNormalStrategy, but the PR description says failing tests should use self.assertRaises. There is no assertRaises usage in this test. If the underlying breeze.stats.DescriptiveStats issue is actually fixed, this is fine, but if it still fails at runtime this will break CI instead of being a controlled expected failure.
| # can not implement breeze.stats.DescriptiveStats, because it is not an interface | ||
| # (breeze.stats.DescriptiveStats is in unnamed module of loader 'app') | ||
| @pytest.mark.xfail(reason="TODO: breeze.stats.DescriptiveStats is in unnamed module of loader 'app'") | ||
| def test_holtWinters(self): |
There was a problem hiding this comment.
The @pytest.mark.xfail decorator was removed from test_holtWinters, but no self.assertRaises was added. Same concern as test_BatchNormalStrategy — if the breeze issue isn't actually resolved, this will cause unexpected test failures in CI.
| # Todo: test anomaly detector | ||
| # Doesn't work in verification suite | ||
| # TODO: test anomaly detector | ||
| # Doesn"t work in verification suite |
There was a problem hiding this comment.
The get_anomalyDetector method references self._jvm which is not defined on the test class (the class has self.spark from setup_pyspark). This should likely be self.spark._jvm.
Issue #, if available:
There are no user-facing changes.
Description of changes:
pytest.mark.xfailmarks and asserting the failing statements withself.assertRaises, except for thetest_anomalyDetectortest which relates to a not implemented feature.test_RelativeRateOfChangeStrategytest.blackformatting to anomaly detection tests.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.