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

Add unit tests to ParselyAPIConnection and minor refactor of it #81

Merged
merged 24 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
32e3101
refactor: remove unnecessary if check
wzieba Oct 19, 2023
1427b85
refactor: pass instance of tracker to APIConnection
wzieba Oct 19, 2023
7ac2eda
fix: make `ParselyAPIConnection` package-private
wzieba Oct 19, 2023
2a2ad97
style: make `ParselyAPIConnection#exception` private
wzieba Oct 19, 2023
013cf67
refactor: extract `purgeEventsQueue` method
wzieba Oct 19, 2023
c9825f1
style: update modifier for `purgeStoredQueue`
wzieba Oct 19, 2023
acbaf7a
build: add MockWebServer dependency
wzieba Oct 19, 2023
41e0d25
tests: introduce `FakeTracker`
wzieba Oct 19, 2023
d1e2c85
tests: request without pixels case
wzieba Oct 19, 2023
a1b9bd8
tests: request with pixels case
wzieba Oct 19, 2023
bae3ca3
Merge branch 'main' into api_connection_unit_tests
wzieba Oct 20, 2023
ab4fe3a
tests: update GET request test to wait for task to finish
wzieba Oct 20, 2023
85b9489
tests: update POST request test to wait for task to finish
wzieba Oct 20, 2023
375237b
style: move url declaration to "given"
wzieba Oct 20, 2023
803198a
tests: add test for purging events queue when successful request is made
wzieba Oct 20, 2023
6ebc6b0
tests: add test for not purging events queue when unsuccessful reques…
wzieba Oct 20, 2023
a8f3861
style: make `url` definition test-wide
wzieba Oct 20, 2023
877aea1
tests: update test names to add missing given condition
wzieba Oct 20, 2023
155a3c6
tests: verify that flush timer is stopped after succesful response
wzieba Oct 20, 2023
b95cde6
tests: verify that flush timer is not stopped after unsuccessful resp…
wzieba Oct 20, 2023
8756b27
tests: change FakeTracker from object to class
wzieba Oct 20, 2023
c8795c0
refactor: change result type of `ParselyAPIConnection` to `Void`
wzieba Oct 20, 2023
cf858aa
style: move tearDown below setUp
wzieba Oct 23, 2023
0f21123
refactor: move JSON pixel payload to a file
wzieba Oct 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions parsely/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ dependencies {
testImplementation 'androidx.test:core:1.5.0'
testImplementation 'org.assertj:assertj-core:3.24.2'
testImplementation 'junit:junit:4.13.2'
testImplementation 'com.squareup.okhttp3:mockwebserver:4.12.0'
}

apply from: "${rootProject.projectDir}/publication.gradle"
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,24 @@

import android.os.AsyncTask;

import androidx.annotation.NonNull;

import java.io.OutputStream;
import java.net.URL;
import java.net.HttpURLConnection;

public class ParselyAPIConnection extends AsyncTask<String, Exception, HttpURLConnection> {
class ParselyAPIConnection extends AsyncTask<String, Exception, Void> {
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved

@NonNull
private final ParselyTracker tracker;
private Exception exception;

public Exception exception;
ParselyAPIConnection(@NonNull ParselyTracker tracker) {
this.tracker = tracker;
}

@Override
protected HttpURLConnection doInBackground(String... data) {
protected Void doInBackground(String... data) {
HttpURLConnection connection = null;
try {
if (data.length == 1) { // non-batched (since no post data is included)
Expand All @@ -46,35 +54,23 @@ protected HttpURLConnection doInBackground(String... data) {

} catch (Exception ex) {
this.exception = ex;
return null;
}
return connection;
return null;
}

protected void onPostExecute(HttpURLConnection conn) {
@Override
protected void onPostExecute(Void result) {
if (this.exception != null) {
ParselyTracker.PLog("Pixel request exception");
ParselyTracker.PLog(this.exception.toString());
} else {
ParselyTracker.PLog("Pixel request success");

ParselyTracker instance = null;
try {
instance = ParselyTracker.sharedInstance();
} catch (NullPointerException ex) {
ParselyTracker.PLog("ParselyTracker is null");
}

if (instance != null) {
// only purge the queue if the request was successful
instance.eventQueue.clear();
instance.purgeStoredQueue();
// only purge the queue if the request was successful
tracker.purgeEventsQueue();

if (instance.queueSize() == 0 && instance.storedEventsCount() == 0) {
ParselyTracker.PLog("Event queue empty, flush timer cleared.");
instance.stopFlushTimer();
}
}
ParselyTracker.PLog("Event queue empty, flush timer cleared.");
tracker.stopFlushTimer();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,9 @@ private void sendBatchRequest(ArrayList<Map<String, Object>> events) {

if (isDebug) {
PLog("Debug mode on. Not sending to Parse.ly");
eventQueue.clear();
purgeStoredQueue();
purgeEventsQueue();
} else {
new ParselyAPIConnection().execute(ROOT_URL + "mobileproxy", JsonEncode(batchMap));
new ParselyAPIConnection(this).execute(ROOT_URL + "mobileproxy", JsonEncode(batchMap));
PLog("Requested %s", ROOT_URL);
}
PLog("POST Data %s", JsonEncode(batchMap));
Expand Down Expand Up @@ -518,10 +517,15 @@ private ArrayList<Map<String, Object>> getStoredQueue() {
return storedQueue;
}

void purgeEventsQueue() {
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
eventQueue.clear();
purgeStoredQueue();
}

/**
* Delete the stored queue from persistent storage.
*/
protected void purgeStoredQueue() {
private void purgeStoredQueue() {
persistObject(new ArrayList<Map<String, Object>>());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package com.parsely.parselyandroid

import androidx.test.core.app.ApplicationProvider
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import org.assertj.core.api.Assertions.assertThat
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.LooperMode
import org.robolectric.shadows.ShadowLooper.shadowMainLooper

@RunWith(RobolectricTestRunner::class)
@LooperMode(LooperMode.Mode.PAUSED)
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
class ParselyAPIConnectionTest {

private lateinit var sut: ParselyAPIConnection
private val mockServer = MockWebServer()
private val url = mockServer.url("").toString()
private val tracker = FakeTracker()

@Before
fun setUp() {
sut = ParselyAPIConnection(tracker)
}

@Test
fun `given successful response, when making connection without any events, then make GET request`() {
// given
mockServer.enqueue(MockResponse().setResponseCode(200))

// when
sut.execute(url).get()
shadowMainLooper().idle();

// then
val request = mockServer.takeRequest()
assertThat(request).satisfies({
assertThat(it.method).isEqualTo("GET")
assertThat(it.failure).isNull()
})
}

@Test
fun `given successful response, when making connection with events, then make POST request with JSON Content-Type header`() {
// given
mockServer.enqueue(MockResponse().setResponseCode(200))

// when
sut.execute(url, pixelPayload).get()
shadowMainLooper().idle();

// then
assertThat(mockServer.takeRequest()).satisfies({
assertThat(it.method).isEqualTo("POST")
assertThat(it.headers["Content-Type"]).isEqualTo("application/json")
assertThat(it.body.readUtf8()).isEqualTo(pixelPayload)
})
}

@Test
fun `given successful response, when request is made, then purge events queue and stop flush timer`() {
// given
mockServer.enqueue(MockResponse().setResponseCode(200))
tracker.events.add(mapOf("idsite" to "example.com"))

// when
sut.execute(url).get()
shadowMainLooper().idle();

// then
assertThat(tracker.events).isEmpty()
assertThat(tracker.flushTimerStopped).isTrue
}

@Test
fun `given unsuccessful response, when request is made, then do not purge events queue and do not stop flush timer`() {
// given
mockServer.enqueue(MockResponse().setResponseCode(400))
val sampleEvents = mapOf("idsite" to "example.com")
tracker.events.add(sampleEvents)

// when
sut.execute(url).get()
shadowMainLooper().idle();

// then
assertThat(tracker.events).containsExactly(sampleEvents)
assertThat(tracker.flushTimerStopped).isFalse
}

@After
fun tearDown() {
ParaskP7 marked this conversation as resolved.
Show resolved Hide resolved
mockServer.shutdown()
}

companion object {
val pixelPayload = """
Copy link
Collaborator Author

@wzieba wzieba Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auto-format doesn't work here :/ I didn't format this by hand because it looks ok (readable) and I'm concerned about future edits of this field (and a need to format again by hand).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (💡): I actually wanted to suggest using test/resources/pixel_payload.json instead. Wdyt? 🤔

{
"events": [
{
"idsite": "example.com"
},
{
"idsite": "example2.com"
}
]
}
""".trimIndent()
}

private class FakeTracker : ParselyTracker(
"siteId", 10, ApplicationProvider.getApplicationContext()
Comment on lines +104 to +105
Copy link
Collaborator Author

@wzieba wzieba Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I wanted to introduce a package private interface (QueueManager?) but due to limitations of Java (interface methods must be public) I couldn't do it. Hence, the fake class extends the whole ParselyTracker.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise (❤️): Actually, I like what you did here, so 👍 !

) {

var flushTimerStopped = false
val events = mutableListOf<Map<String, Any>>()

override fun purgeEventsQueue() {
events.clear()
}

override fun stopFlushTimer() {
flushTimerStopped = true
}
}
}