Markbind: NPM script to replace expected output for test site

[marvinchin]

When working features that change the results of the functional tests, I find that I often have to copy the newly generated files from _site to expected (and then view the git diff to verify the changes made are correct).

I was wondering if any other devs have the same workflow, and if adding an npm command (e.g. npm run test-update-site) to perform the replacement operation would be useful.

[Xenonym]

Just a thought: perhaps this npm command should allow us to review changes to the expected site before copying the new _site over?

[damithc]

Just a thought: perhaps this npm command should allow us to review changes to the expected site before copying the new _site over?

This can be done using git diff, right? If the change is not as expected, can revert.

[sijie123]

I've definitely experienced the same annoyance when I make changes to the test site. Sometimes I do wish for such a tool to exist.
That said, from a software engineering perspective, I feel like the entire reason tests exist is to protect us from regressions and give us confidence that our code works correctly. If we create a tool that simply copies current output to expected output, I feel like we will lose the purpose of the test.

So while such a tool is really nice and convenient, I wonder if this feature should be added. Actually, I wonder how large companies write tests for components that change so rapidly... Maybe someone with more experience can give their insights?

[marvinchin]

Definitely agreed that we shouldn't just be copying the output to expected and committing it blindly 😅.

What I had in mind was to use the script to easily update the expected folder when some change is expected and view the diff in git to verify the changes are what is expected before committing the change. Otherwise, if no change is expected, we should not be running the update command anyway so we should still be protected from unintended changes to output.

For reference, Jest snapshots work the same way.

[damithc]

Good observations @sijie123
We went through a similar scenario in TEAMMATES where we do a lot of HTML comparison tests, which means devs faced the same annoyance as cited here.
At some point, devs created a GodMode for running tests which essentially does what is proposed here. The PR reviewers were supposed to check the diff to ensure the changes to 'expected' files (done by the GodMode) are as expected.
Over time, it turned out that PR reviewers rarely, if ever, looked at those diffs, mainly because those diffs were huge.
Recently they have abolished the GodMode and are implementing a replacement, which is similar in nature. But this time they are trying to minimize such comparison tests so that the diffs generated will be smaller and are practical to eyeball. It still relies on PR reviewers to inspect the diff to confirm the new test files generated by the tool are as expected. It remains to be seen if the reviewers can maintain such discipline -- the new tool has not been put into practice yet.

[damithc]

For reference, Jest snapshots work the same way.

I believe TEAMMATES' GodMode replacement uses Jest snapshots too.

[marvinchin]

In light of the concerns raised, and with the hindsight of what challenges TEAMMATES is facing with such an approach, is there still interest in having such a script? 😅

[yamgent]

I think having the script should be fine; even if you do it manually, it does not necessary mean that you will be more disciplined in checking the final output compared to doing to automatically with script.

We should also strive not to make the diff too difficult to read, by breaking down the pages of the test sites into smaller components, and not using fancy formatting as much possible. I have commented these before on someone's PR (that the test's HTML output being a bit messy to read sometimes), but we can explore this issue further separately.

[wkurniawan07]

TEAMMATES chief maintainer here.

At some point, devs created a GodMode for running tests which essentially does what is proposed here. The PR reviewers were supposed to check the diff to ensure the changes to 'expected' files (done by the GodMode) are as expected.

Over time, it turned out that PR reviewers rarely, if ever, looked at those diffs, mainly because those diffs were huge.

Recently they have abolished the GodMode and are implementing a replacement, which is similar in nature. But this time they are trying to minimize such comparison tests so that the diffs generated will be smaller and are practical to eyeball.

The reason of such abolishment is not so much on "the [old] diffs are huge" and "the [new] diffs generated will be smaller and are practical to eyeball".

The reason is because God mode has been misused to the point that the value of those tests is practically zero now. Consider this case:

1. You have a starting web page
2. You click something on the web page, it changes
3. You have a web page which is different from the starting

God mode previously made expected outputs for both (1) and (3). The testing value is suddenly gone because you do not know what is different between those two. This is not just one-off occurrence. This is misused to the level that it becomes an anti-pattern, and the result is more than 80% of such "tests" have no value right now.

Jest snapshot testing works with the same principle (if not close to identical), however:

• It supports testing at component level (at least for React and Angular)
• It does not even need the web page to be rendered which is a huge cost saving
• By extension of the previous point, it does not need any real data

The level in which Jest runs solves the problem of "huge diffs".

However, it remains a problem that Jest can be misused the way God mode was misused before. The resolution here is to restrict where snapshot testing can be used:

1. To generate a snapshot when given a component and its internal data, snapshot testing is appropriate
2. To examine how the page is different between interactions, snapshot testing IS NOT to be used

Although I hope this will solve the problem for good (it seems that way for now), as rightfully said, "It remains to be seen if the reviewers can maintain such discipline".

[wkurniawan07]

If I understand Markbind correctly, snapshot testing will be appropriate and is less prone to mistakes done in TEAMMATES because there is not much page interactions (especially those which fetch/manipulate data from some source) going on. However, you still need to decide when and how to use such test such that it will actually have value.

[yamgent]

If I understand Markbind correctly, snapshot testing will be appropriate and is less prone to mistakes done in TEAMMATES because there is not much page interactions (especially those which fetch/manipulate data from some source) going on. However, you still need to decide when and how to use such test such that it will actually have value.

Thanks for your valuable insight @wkurniawan07! 😊