Corrupted converted Excel files

[dionissds]

After the latest changes in dev branch, our integration tests Xliff-to-File fail due to corrupted converted Excel files. I suspect the cause may be the changes in this ticket 1051 or any other ticket related to the OpenXml filter, such as described in this PR.

First, I got these two differences in the tests, related to the column names, the first one in an Xliff with no targets translated and the second one with correctly translated targets:

Checking the lastest commits merged into dev, I saw ticket 1051 that seemed like a good match. I reverted its commit, 9cf885f73e094d4059a4b9bb8eb4e99fe9c2f5e9, and I managed to get rid of the above problems, but then run into these ones:

The main problem is not only on the above differences but that the Excel files themselves are corrupted when trying to open in Excel. As the Excel file is just a bunch of archived XMLs, our comparison in integration tests is done by fetching the excel, naming it to .zip, then unzipping the archive and comparing each XML file within the archives.

We need to allow the empty targets, for which we worked and discussed in this PR. In fact, this issue I’m addressing now is caused after we reverted the changes from the mentioned PR that caused the failing Okapi’s tests and merged your latest dev branch version. So, for us this issue is in the same scope as the PR, we need to allow the empty targets, by making sure that our integration tests do not fail (like above) when we merged Okapi’s latest changes.

If relevant: we are using RawDocumentToFilterEventsStep and then MergingStep.

Attached are the Xliff files we are using as input, and the expected Xlsx.
Let me know what other information you need from us, any help is highly appreciated as we are stuck here for many days now.

[Jim Hargrave]

@Dionis D @YvesS @Chase Tingley Looks like there is more code out there that needs to be upgraded to use the TextUnitMerger. Can you see if you can integrate that somewhere? it would just be a way to get your mergedTU and then you can use the existing code to update alttrans of whatever you need.

I think what we should do is come up with a new IMergerStep (with possible Abstract base class) interface that an enforce some of these standards.

[dionissds]

Apparently, this was an issue with the Excel files themselves - they were corrupted

Sorry for bringing this up here, it’s solved now on our side, so I’m closing this issue.

‌