Error message on `cli`, `core` tests

[gerteck]

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

No response

Tell us about your environment

Win 11, Ubuntu GH Actions

MarkBind version

5.3.3

Describe the bug and the steps to reproduce it

CLI Tests

There are error messages in the MarkBind packages/cli tests. See:

• I am running this on master branch, individually through cd packages/ cli; npm run test. Due to the tests running in parallel, if you run npm run test from root, lerna will run tests in parallel, so test messages are warped, makes debugging hard. So best to run each test locally individually.

Additionally, in the latest PR check, this 'error' also shows.
https://github.com/MarkBind/markbind/actions/runs/13458086184/job/37606471167#step:8:233

Core Tests

There are also error messages when running core tests.

i.e. cyclic reference, does not exist.

Issue

This error is misleading. Is this supposed to be expected behaviour? If it is, could the error be caught and not shown?

• I suspect this is the case for core tests.

Otherwise, is the test actually failing in some segment, but reported as pass?

• I suspect this might be the case for cli tests, hopefully not.

Expected behavior

No response

Anything else?

No response

[AgentHagu]

Otherwise, is the test actually failing in some segment, but reported as pass?
- I suspect this might be the case for cli tests, hopefully not.

After some investigation, the error during CLI testing appears to be intentional. Firstly, the error message comes from the implementation of the src attribute for popover elements. This behavior was introduced in PR #1780. The purpose of the error was to prevent URLs like "https://www.google.com/" from being used in the src attribute.

The specific test code triggering the error is as follows:

<popover header="URLs are not valid" src="http://www.shouldnotwork.com">
  URLs should not be valid
</popover>

This popover deliberately triggers the error and the snapshot test expects the popover's HTML content to be replaced with an error message like this:

I do agree that the error is misleading though. The CLI tests do not verify if the error is properly logged, so perhaps we can switch from logging this as an error to logging it as a warning instead.

Users will still be notified and developers can ignore the warnings during testing. A PR could also be created to improve the error message since the current one is not user-friendly anyways.

EDIT:
I've made another suggestion for a fix in PR #2639 that logs the error as a warning for developers during testing but logs it as an error for users.

I haven't looked into the core package tests yet but I suspect the errors (at least those in the screenshots) are also from the same file and a similar fix can be applied to them as well.

[tlylt]

I think these logs are correct because we tested error cases and these logs are generated due to intentional incorrect inputs in those test cases. They are indeed misleading for new devs, so if we can improve it we should.