After investigation, tests fail in this PR due to a mismatch between expected files and generated files. We can refer to this PR where the relevant changes were made. This is because in packages/cli/test/functional/testSites.js, a list of plantuml files that are expected in test-site are saved, and in the compare function the existence of these ignored files is checked for before all ignored files are filtered out and compared.
However, in this line compare(siteName, 'expected', '_site', siteName === 'test_site' ? plantumlGeneratedFiles : []); essentially plantuml files are only checked for in test_site.
This is not scalable for other generated images in sites that aren't test-site, and means that this image cannot be ignored as per the instructions in the dev-guide without breaking CI workflows. This was not discovered until now because PUML files were not tested outside of test-site before, but since we currently maintain test sites for the templates which are available for users (and it is not reasonable to expect us not to have PUML in these templates), we should change the status quo.
Expected behavior
We should rewrite this section to allow for files to be ignored in any section. We should also leave a comment in the .gitignore and/or in the dev guide to also update a list within the site test files which is passed into this function.
Perhaps using a dictionary with keys being the site_names and the values being an array of ignored files. Then the value passed into compare can be something like:
ignoredFilesList = ignoredFiles.site_name if site_name in ignoredFiles.keys() else []
Please confirm that you have searched existing issues in the repo
Yes, I have searched the existing issues
Any related issues?
https://github.com/MarkBind/markbind/pull/2497
Tell us about your environment
M2 Mac v13.6.6
MarkBind version
5.4.0 (latest)
Describe the bug and the steps to reproduce it
After investigation, tests fail in this PR due to a mismatch between expected files and generated files. We can refer to this PR where the relevant changes were made. This is because in
packages/cli/test/functional/testSites.js, a list of plantuml files that are expected in test-site are saved, and in the compare function the existence of these ignored files is checked for before all ignored files are filtered out and compared.However, in this line
compare(siteName, 'expected', '_site', siteName === 'test_site' ? plantumlGeneratedFiles : []);essentially plantuml files are only checked for in test_site.This is not scalable for other generated images in sites that aren't test-site, and means that this image cannot be ignored as per the instructions in the dev-guide without breaking CI workflows. This was not discovered until now because PUML files were not tested outside of test-site before, but since we currently maintain test sites for the templates which are available for users (and it is not reasonable to expect us not to have PUML in these templates), we should change the status quo.
Expected behavior
We should rewrite this section to allow for files to be ignored in any section. We should also leave a comment in the .gitignore and/or in the dev guide to also update a list within the site test files which is passed into this function.
Perhaps using a dictionary with keys being the site_names and the values being an array of ignored files. Then the value passed into compare can be something like:
(syntax not accurate at all).
Anything else?
No response