Fix all date constructors to Date(0) to prevent test flakiness

[mikiher]

Brief summary

Fix Date constructors to Date(0) to provide stable dates and prevent test flakiness

Which issue is fixed?

No issue.

In-depth Description

Using just Date() caused the test input to be unstable, and caused sporadic test failures.

How have you tested this?

Ran the test to make sure it still passes.

[advplyr]

I don't think we want to add Date(0) to all of them, only the duplicates that should be removed.

The issue happens because of this ORDER BY updatedAt sometimes returning the duplicate series to get removed.

audiobookshelf/server/migrations/v2.15.0-series-column-unique.js

Lines 124 to 139 in 565eb42

	const [mostRecentSeries] = await queryInterface.sequelize.query(
	`
	SELECT id
	FROM Series
	WHERE name = :name AND libraryId = :libraryId
	ORDER BY updatedAt DESC
	LIMIT 1
	`,
	{
	replacements: {
	name: duplicate.name,
	libraryId: duplicate.libraryId
	},
	type: queryInterface.sequelize.QueryTypes.SELECT
	}
	)

[nichwall]

Am I correct that Date(0) just sets the date to the 1970 epoch? If so we could also use Date(0) for everything to keep and then something else like Date(7) to force the time to be later.

[mikiher]

Am I correct that Date(0) just sets the date to the 1970 epoch? If so we could also use Date(0) for everything to keep and then something else like Date(7) to force the time to be later.

Yes, that is correct.

I don't think we want to add Date(0) to all of them, only the duplicates that should be removed.

Sorry, I might have been hasty to put Date(0) on all of them, even though the test passes.

But we definitely cannot leave the no-parameter Date constructor. It is not good practice to have a unit test where the input changes every time we run it. It is is called a test fixture for a reason.

[advplyr]

In this case we are testing that the series most recently updated is the one that was kept. new Date(0) will always be less than new Date() but I agree hard coding is better.

[advplyr]

I was able to reproduce the failure on the other issue but only after like 50 times running it. Best to write a loop if you want to reproduce

[nichwall]

I wrote a script that continuously runs the tests and was also able to reproduce the flakiness of this migration test. I believe I have fixed the flakiness of this migration, but found another flaky test in the LibraryItemController test. I will take a stab at fixing that unit test as well and open a new PR with the fixes (after running continuously for a while to make sure I don't uncover any more flaky tests).

  LibraryItemController
    checkRemoveAuthorsAndSeries
      1) should remove authors and series with no books on library item delete
      ✔ should remove authors and series with no books on library item batch delete
      ✔ should remove authors and series with no books on library item update media

[nichwall]

I was not able to figure out the flaky LibraryItemController test, but I did open #4055 to fix the 2.15.0 test being flaky.

The library item delete and library item batch delete both appear to be flaky.