Skip to content

Create tests for the most common use cases#17

Merged
deBhal merged 2 commits intomasterfrom
develop
Jul 8, 2020
Merged

Create tests for the most common use cases#17
deBhal merged 2 commits intomasterfrom
develop

Conversation

@alshakero
Copy link
Copy Markdown
Member

@alshakero alshakero commented Apr 16, 2020

Hi,

This PR proposes a few tests for the snapshot.js file. This will hopefully make the development process a little safer and more rigorous.

Once this PR is approved, or more tests are suggested, we could then send minor patches such as #16 without less risk on affecting the service stability.

Soon, after this, I'll add a failing test for #16, then I'll send a patch to make it pass.

Thanks!

@deBhal
Copy link
Copy Markdown
Contributor

deBhal commented Jul 8, 2020

Looks good to me! I'm going to go ahead and merge with two notes:

I spoke with @darthhexx yesterday, and he stressed the importance of tests covering the php<->js and back because the php does additional processing on the image, so it'd be great if we could add those.

Having said that, it's not an either/or scenario, and these tests still seem valuable on their own merit, so I'm going to go ahead and merge this PR into master (we can always revert later if we need to).

The other thing is that I'm still a bit scarred from maintaining tests with sleep()s, so I've pushed #20 that favors listening to messages from the queue. This doesn't make a huge difference immediately, so I've made a new PR instead of updating this one. I'd love a review!

@deBhal deBhal merged commit 748a64f into master Jul 8, 2020
@simison simison deleted the develop branch August 7, 2020 08:03
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.

2 participants