Bug: Markdown Message Ordering got shuffled?

[fbartho]

Example code:

	markdown(assembleTable(recentBuildHeaders, recentBuildLinks));
	markdown(
		makeDisclosableBlock(
			`All commits with builds for ${currentBranchName}`,
			assembleTable(allBuildHeaders, allBuildLinks),
		),
	);
	markdown(
		"#### Latest Build Status Pages\n" +
			assembleTable(buildBadgeTableHeaders, [buildBadgeTableValues]),
	);

This code hasn't been changed since 2019-03-29

Expected Rendering (from last week):

Yesterday's Rendering:

If you compare the code, to the renderings, the "Latest Build Status Pages" table is expected to be last.

It appears we're using [email protected] -- will try updating, to see what happens.

[fbartho]

Updating to [email protected] did not resolve this issue. It appears that markdown entries are being flipped in order when rendering to a comment.

[orta]

It should be deterministic - so maybe it's just deterministically reversed.

[fbartho]

The problem I see is that based on the code, it’s not deterministic - otherwise I could fix this! The top entry of 3 (in code) is shuffled to the bottom (in screenshot 2)

[orta]

Ah, fair - I guess it's not unreasonable to record the order when a violation is created to ensure it's consistent on both sides.

I'm pretty sure there are some tests which might fail if it's indeterminate, but it's been a while since I've looked at this codebase

[fbartho]

I could rewrite my code, to combine these markdown messages into one call, so I have a workaround for myself. Not sure if this is valuable to try to track down?

[orta]

Looks like it's been around while https://github.com/danger/danger-js/issues/845 - so if no-one else has also thought it important enough to work on, that's fine IMO

[falkenhawk]

Is there a possibility to fix this issue? 🙏

[falkenhawk]

my code looks like this:

function doStuff(n) {
  return Promise.all([...]).then(res => {
     markdown(`message ${n}`);
  });
}

schedule(async () => {
  await doStuff(1);
  await doStuff(2);
  await doStuff(3);
});

and testing locally with danger pr always yields: (maybe i was just lucky to get the correct order in all test runs 🤔)

## Markdowns
message 1
message 2
message 3

while running this in github workflow, the resulting order of the markdown strings are non-deterministic. Sometimes it's in correct order, sometimes it's shuffled.

ofc my real doStuff function is much more complex and actually comes from an external plugin, so not much there for me to hijack the messages and concatenate them manually before calling markdown() once :(

EDIT: it seems to affect not only markdown() but also message()/warn() in the same way

[orta]

You're welcome to fix it 👍

Maybe add a creation date to the Violations and then sort it at the end

[falkenhawk]

You're welcome to fix it 👍

#1336 @orta