Make sorting tests work with remote queries & variant analysis history items

[elenatanasoiu]

🚨 Please review this commit-by-commit.

This is a follow-up to #1688 where we first started converting our query history tests to check all types of history items instead of just local query ones.

In order to test sorting with local queries, remote queries and variant analyses, we've had to make a few changes:

1. Allow createMockVariantAnalysis and createMockVariantAnalysisHistoryItem to receive named params so that we can set resultCount and executionStartTime. We need this in order to test sorting by result and date.
2. We do the same for createMockRemoteQueryHistoryItem.
3. We're combining the two existing factory methods for creating a LocalQueryInfo into a single createMockLocalQueryInfo method.
4. For createMockLocalQueryInfo we're now returning a real object instead of typecasting something else. Not having a real object blocked us from being able to understand which fields we need in our tests and how they interact together.
5. Finally, we change our sorting tests to check all types of history item types, instead of just local queries.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[koesie10]

LGTM, thanks for the multiple commits, those really make it easier to review!

[koesie10]

Thanks for making this change, this makes it much easier to use!

An alternative to this would be to allow passing in all properties using something like this:

export function createMockVariantAnalysis(data: Partial<VariantAnalysis>): VariantAnalysis {
  return {
    status: VariantAnalysisStatus.InProgress,
    // ... other defaults
    ...data,
  };
}

What do you think?

[elenatanasoiu]

Nice! I gave it a shot but I think it would involve declaring these params:

scannedRepos = createMockScannedRepos(),
skippedRepos = createMockSkippedRepos(),

outside of the method call and passing them in. That would require a lot more refactoring and I'm a bit refactored out. 😄

I considered adding it any way:

export function createMockVariantAnalysis({
  status = VariantAnalysisStatus.InProgress,
  scannedRepos = createMockScannedRepos(),
  skippedRepos = createMockSkippedRepos(),
  executionStartTime = faker.datatype.number(),
  data,
}: {
  status?: VariantAnalysisStatus,
  scannedRepos?: VariantAnalysisScannedRepository[],
  skippedRepos?: VariantAnalysisSkippedRepositories,
  executionStartTime?: number | undefined,
  data?: Partial<VariantAnalysis>
}): VariantAnalysis {
  return {
    // all the existing stuff
    ...data
  }
}

but that seems like it would defeat the point of it.

[elenatanasoiu]

Ok I've thought this a bit more. I'll try in my next PR to see if we could adapt it.

[elenatanasoiu]

Thanks for the review @koesie10 !