- 
                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.