Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add long running test #795

Merged
merged 4 commits into from Mar 28, 2022
Merged

chore: add long running test #795

merged 4 commits into from Mar 28, 2022

Conversation

Caele
Copy link
Collaborator

@Caele Caele commented Mar 16, 2022

Motivation

Adds a test for long runnning queries by adding a delay to the mocked engine calls. It currently doesn't properly cancel the call when you press cancel (or the test presses cancel), but it covers the functionality.

@streamside Feel free to clean up this code if you like, had to do some light hacking to get the delay to work without slowing everything down too much.

@Caele Caele requested a review from streamside March 16, 2022 12:42
@Caele Caele marked this pull request as ready for review March 16, 2022 12:42
@streamside
Copy link
Contributor

The intent when writing this was to include all properties on the genericObjects passed to the enigma mocker. The property delay is more of a setting on how it should behave. Down the road there might be conflicts if engine also provides a property with the same name. I suggest to introduce an options object - which for now includes only the delay setting - to the exposed API:

fromGenericObjects(genericObjects, options)

The delay option would be "global" and effect all generic objects being mocked. If more granular control is desired the dev would be able to delay individual objects / functions.

The code looking up the generic object by its qId didn't become that pretty with the findAsync(). Another option would be to return the qId along with the mock when creating the mocks (around line 32 in get-object-mock.js). The lookup to find the correct object would become easier and no need to add qId on the mock.

Let me know your thoughts about it and if I should make any changes to the code.

@streamside
Copy link
Contributor

@Caele I have updated the PR with two changes. (1) delay added in options object the enigma-mocker which effects all async calls, and (2) lookup of the mocks via qId is simplified. Feel free to take a look!

@streamside streamside merged commit 0ec0078 into master Mar 28, 2022
@streamside streamside deleted the tsm/long-running branch March 28, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants