Unfail repository tests#251
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -1,13 +1,13 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
nit: from py4j.protocol import Py4JError is imported but pytest was removed. However, Py4JError is used in the test, so this is fine. Just noting that pytest removal is correct since pytest.mark.xfail is no longer used.
| .saveOrAppendResult(resultKey) | ||
| .run() | ||
| with self.assertRaises(Py4JError) as err: | ||
| _ = ( |
There was a problem hiding this comment.
The test_fail_no_load test asserts the error message string exactly, but MetricsRepository._check_RepositoryLoader checks if not self.RepositoryLoader which would raise AttributeError only if RepositoryLoader was never set as an attribute. However, repository.before(...) calls self._check_RepositoryLoader() which accesses self.RepositoryLoader. Since RepositoryLoader is never initialized in __init__ of FileSystemMetricsRepository (or its parent MetricsRepository), this will raise AttributeError: 'FileSystemMetricsRepository' object has no attribute 'RepositoryLoader' — but this is an implementation detail that could change. The assertion is correct for the current code, but it's brittle. Consider using self.assertIn or just self.assertRaises(AttributeError) without checking the exact message.
| """This test should fail because it doesn't call useRepository() before saveOrAppendResult()""" | ||
| """This run fails because it doesn't call useRepository() before saveOrAppendResult().""" | ||
| metrics_file = FileSystemMetricsRepository.helper_metrics_file(self.spark, "metrics.json") | ||
| print(f"metrics filepath: {metrics_file}") |
There was a problem hiding this comment.
test_fail_no_useRepository asserts a specific Py4J error message substring "Method saveOrAppendResult([class com.amazon.deequ.repository.ResultKey]) does not exist". This is fragile because the exact error message depends on the Py4J and Deequ JAR versions. If the Deequ version changes, this message could differ. Consider just asserting self.assertRaises(Py4JError) without checking the message content, or use a less specific substring like "saveOrAppendResult".
1 participant
Issue #, if available:
There are no user facing changes.
Description of changes:
pytest.mark.xfailmarks and asserting the failing statements withself.assertRaises.blackformatting to repository tests.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.