Unit test bug - in testing utils.convert, instanceof Date operation doesn't robustly capture parsing

[strazto]

Migrated from #663 (comment)

Changing these test-cases raised another concern:

I'm not convinced that the instance of Date operation is actually robust enough to correctly test for valid parsing:

vis-timeline/test/util-convert.test.js

Lines 48 to 50 in f95eb73

	it("converts to Date from String", function() {
	assert(util.convert("1198908717056", "Date") instanceof Date);
	});

Interactively (via devtools) i found

new Date("1198908717056")
// Invalid Date
new Date("1198908717056").valueOf()
// NaN
new Date("1198908717056") instanceof Date
// true
timeline.options.moment(new Date("1198908717056")).toDate() 
//Invalid Date
timeline.options.moment(new Date("1198908717056")).toDate() instanceof Date
// true

isNaN(timeline.options.moment(new Date("1198908717056")).toDate().valueOf())
// true

I don't know for sure what the expected value of util.convert("1198908717056", "Date") is, but I suspect that this test case assumes that it should be equal to util.convert(1198908717056, "Date") .

Expected behaviour

When checking Date parsing outputs, we need to check that the output is actually a valid date.
Checking this via .valueOf() and !isNaN() might be a good approach.

Also, I'm right about this, then fixing this test case means adding failing test cases, which breaks the build.

This implies that the parser needs to be fixed to accept strings, or that this test case was never correct.

Environment

Chrome, Ubuntu 18.04

[vis-bot]

🎉 This issue has been resolved in version 7.3.10 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀