-
Notifications
You must be signed in to change notification settings - Fork 7
Re-organizing test resources into packages #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## dev #525 +/- ##
============================================
+ Coverage 77.85% 77.88% +0.03%
+ Complexity 2152 2148 -4
============================================
Files 275 275
Lines 8493 8493
Branches 807 807
============================================
+ Hits 6612 6615 +3
- Misses 1483 1486 +3
+ Partials 398 392 -6
Continue to review full report at Codecov.
|
… testcontainersFiles)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work, especially on the code quality with test containers. However, I have some remarks left for you.
src/test/groovy/edu/ie3/datamodel/io/source/csv/CsvTestDataMeta.groovy
Outdated
Show resolved
Hide resolved
src/test/groovy/edu/ie3/datamodel/io/source/csv/CsvTestDataMeta.groovy
Outdated
Show resolved
Hide resolved
src/test/groovy/edu/ie3/datamodel/io/source/sql/SqlWeatherSourcePsdmIT.groovy
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Cleaning up the structure a bit more
This comment has been minimized.
This comment has been minimized.
# Conflicts: # src/test/groovy/edu/ie3/datamodel/io/source/sql/SqlTimeSeriesMappingSourceIT.groovy
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
# Conflicts: # src/test/groovy/edu/ie3/datamodel/io/source/sql/SqlTimeSeriesMappingSourceIT.groovy # src/test/groovy/edu/ie3/datamodel/io/source/sql/SqlWeatherSourceCosmoIT.groovy # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_c.sql # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_h.sql # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_p.sql # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_ph.sql # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_pq.sql # src/test/resources/edu/ie3/datamodel/io/source/sql/_timeseries/time_series_pqh.sql
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good ! Thanks a lot :)
Fixed the last comment, I think this is fine now
Resolves #512
Resolves #559
Additional benefit: Within
CsvTestDataMeta, now all needed resources are resolved there. Before, sometimes resource directory paths were appended with suffixes, so that it wasn't checked whether the resources were actually available. Because of this, some tests did not fail that should've failed. This is now fixed.