gemmellr commented on code in PR #4388:
URL: https://github.com/apache/activemq-artemis/pull/4388#discussion_r1123002887
##########
tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/upgradeTest/CompareUpgradeTest.java:
##########
@@ -149,7 +149,12 @@ private void compareDirectories(String expectedFolder,
String upgradeFolder, Str
String expectedString =
expectedIterator.next().replace("Expected", "").trim();
String upgradeString = upgradeIterator.next().trim();
- Assert.assertEquals("error on line " + line + " at " +
upgradeFile, expectedString, upgradeString);
+
+ if (f.getName().endsWith("xml") &&
expectedString.trim().startsWith("<!--") &&
upgradeString.trim().startsWith("<!--")) {
+ logger.debug("Both {} and {} are comments, which I don't
really mind what's going on inside the comment", expectedString, upgradeString);
Review Comment:
I think the comments of files we are not actually upgrading will change
infrequently enough and trivially enough (given they are comments), that it
would be better just to update the original thing being compared in this case
so it matches the post-update expectations, and leave the test handling as it
is rather than trying to make it clever enough to ignore comments but not break
other necessary checks. We have each all spent longer already than doing that
would have taken.
(We could presumably instead even make the upgrader upgrade this
comment-only-changed file so it matched the newly generated expectations file.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
