Add integration test and utils #2482
Conversation
| * filename | ||
| * @return the batch task if successful or null on error | ||
| */ | ||
| public static @Nullable BatchTask runBatch(BatchQueue newQueue, @NotNull MZmineProject project, |
There was a problem hiding this comment.
maybe rename to runBatchQueue and the other to runBatchFile - With so many arguments I had to search for the difference.
| final TaskStatus status; | ||
| // create ThreadPool | ||
| if (currentStepTasks.size() > 1) { | ||
| logger.finest(() -> "Processing " + currentStepTasks.size() + " tasks in the thread pool."); |
There was a problem hiding this comment.
maybe remove? also the loggin below?
| @NotNull Instant moduleCallDate) { | ||
| super(storage, moduleCallDate); | ||
|
|
||
| logger.info("Initializing smoothing task"); |
There was a problem hiding this comment.
maybe remove? also the other debug logs in this class?
| public static CSVExportModularParameters create(File csvExportFile, | ||
| FeatureListRowsFilter rowsFilter, boolean omitEmpty, String idSeparator, | ||
| FeatureListsSelection featureListsSelection) { | ||
| final ParameterSet parameters = new CSVExportModularParameters().cloneParameterSet(); |
There was a problem hiding this comment.
fieldSeparator is not set. Maybe set it to the default or add argument to metdho
| for (Entry<Integer, RowsRelationship> e : corrMap.entrySet()) { | ||
| final List<Entry<Integer, RowsRelationship>> entries = corrMap.entrySet().stream() | ||
| .sorted(Comparator.comparingInt(Entry::getKey)).toList(); | ||
| for (Entry<Integer, RowsRelationship> e : entries) { |
There was a problem hiding this comment.
this is still from the unmerged PR maybe revert to:
for (Entry<Integer, RowsRelationship> e : corrMap.entrySet()) | static void initMzmine() { | ||
| MZmineTestUtil.startMzmineCore(); | ||
| ProjectService.getProjectManager().clearProject(); | ||
| ProjectService.getProject().clearSpectralLibrary(); |
There was a problem hiding this comment.
thats maybe a great thing to have in a MZmineTestUtil.clearAll
This way we can use it in other tests as well and makes us remember to clear the libraries as well
mzmine-community/src/test/java/integrationtest/IntegrationTests.java
Outdated
Show resolved
Hide resolved
mzmine-community/src/test/java/integrationtest/IntegrationTests.java
Outdated
Show resolved
Hide resolved
| * @return a list of CheckResult objects containing the comparison results; the list is empty if | ||
| * all comparisons are successful | ||
| */ | ||
| public static List<CheckResult> runBatchCompareToCsv(URL batchFile, URL baseCsvFile, File tempDir, |
There was a problem hiding this comment.
unused?
For readability I think it is better to not put very complex ?: stream operations into method calls and constructor calls.
It is quite hard to see what belongs to what.
I think putting the below into a separate line before the call or putting it in a method that describes what happens is better:
overrideDataFiles != null ? Arrays.stream(overrideDataFiles).filter(Objects::nonNull).map(IntegrationTestUtils::urlToFile).toArray(File[]::new) : null
robinschmid
left a comment
There was a problem hiding this comment.
Looks very nice already.
I think some of the complexity in IntegrationTestUtils could be avoided if an IntegrationTest class is added that configures the test and maybe even the expected results.
| .rawFiles("171103_PMA_TK_QC_04-4to5min.mzML", "171103_PMA_TK_QC_05-4to5min.mzML") | ||
| .specLibsFullPath("spectral_libraries/integration_tests/massbank_nist_for_tests.msp", | ||
| "spectral_libraries/integration_tests/MoNA-export-LC-MS-MS_Spectra.json").build() | ||
| .runBatchGetCsvFile(); |
There was a problem hiding this comment.
great addition running the function there and getting the file directly.
Runs a batch file, exports a modular .csv and compares it to a expected results table using the CompareModularCsv module