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

Refactor End-to-End (e2e) PULL Transfer Tests #4727

Open
AndrYurk opened this issue Jan 15, 2025 · 0 comments · May be fixed by #4733
Open

Refactor End-to-End (e2e) PULL Transfer Tests #4727

AndrYurk opened this issue Jan 15, 2025 · 0 comments · May be fixed by #4733
Labels
feature_request New feature request, awaiting triage triage all new issues awaiting classification

Comments

@AndrYurk
Copy link
Contributor

AndrYurk commented Jan 15, 2025

Feature Request

Refactor End-to-End (e2e) PULL Transfer Tests to avoid code duplication

Which Areas Would Be Affected?

TransferPullEndToEndTest class

Why Is the Feature Desired?

There is significant code duplication, especially in the following tests:

httpPull_dataTransfer_withEdrCache
suspendAndResumeByConsumer_httpPull_dataTransfer_withEdrCache
suspendAndResumeByProvider_httpPull_dataTransfer_withEdrCache
pullFromHttp_httpProvision

var transferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();
CONSUMER.awaitTransferToBeInState(transferProcessId, STARTED);

var transferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();
CONSUMER.awaitTransferToBeInState(transferProcessId, STARTED);

var consumerTransferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();
CONSUMER.awaitTransferToBeInState(consumerTransferProcessId, STARTED);

var consumerTransferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();
CONSUMER.awaitTransferToBeInState(consumerTransferProcessId, STARTED);

var transferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();

var transferProcessId = CONSUMER.requestAssetFrom(assetId, PROVIDER)
.withTransferType("HttpData-PULL")
.execute();

var edr = await().atMost(timeout).until(() -> CONSUMER.getEdr(transferProcessId), Objects::nonNull);
// Do the transfer
var msg = UUID.randomUUID().toString();
await().atMost(timeout).untilAsserted(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data")));

var edr = await().atMost(timeout).until(() -> CONSUMER.getEdr(transferProcessId), Objects::nonNull);
var msg = UUID.randomUUID().toString();
await().atMost(timeout).untilAsserted(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data")));

var secondEdr = await().atMost(timeout).until(() -> CONSUMER.getEdr(transferProcessId), Objects::nonNull);
var secondMessage = UUID.randomUUID().toString();
await().atMost(timeout).untilAsserted(() -> CONSUMER.pullData(secondEdr, Map.of("message", secondMessage), body -> assertThat(body).isEqualTo("data")));

var edr = await().atMost(timeout).until(() -> CONSUMER.getEdr(consumerTransferProcessId), Objects::nonNull);
var msg = UUID.randomUUID().toString();
await().atMost(timeout).untilAsserted(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data")));

var secondEdr = await().atMost(timeout).until(() -> CONSUMER.getEdr(consumerTransferProcessId), Objects::nonNull);
var secondMessage = UUID.randomUUID().toString();
await().atMost(timeout).untilAsserted(() -> CONSUMER.pullData(secondEdr, Map.of("message", secondMessage), body -> assertThat(body).isEqualTo("data")));

await().atMost(timeout).untilAsserted(() -> {
var edr = await().atMost(timeout).until(() -> CONSUMER.getEdr(consumerTransferProcessId), Objects::nonNull);
CONSUMER.pullData(edr, Map.of("message", "some information"), body -> assertThat(body).isEqualTo("data"));
});

// checks that the EDR is gone once the contract expires
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.getEdr(transferProcessId)));
// checks that transfer fails
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data"))));

// checks that the EDR is gone once the transfer has been suspended
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.getEdr(transferProcessId)));
// checks that transfer fails
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data"))));

// checks that the EDR is gone once the transfer has been suspended
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.getEdr(consumerTransferProcessId)));
// checks that transfer fails
await().atMost(timeout).untilAsserted(() -> assertThatThrownBy(() -> CONSUMER.pullData(edr, Map.of("message", msg), body -> assertThat(body).isEqualTo("data"))));

This duplication is likely to increase as new tests are added, making the codebase harder to maintain and understand.

Solution Proposal

If possible, provide a (brief!) solution proposal.
Create private methods to handle the duplicated code, reuse them across tests, and establish a clearer structure. This approach will make the code more readable, maintainable, and compact.

@AndrYurk AndrYurk added feature_request New feature request, awaiting triage triage all new issues awaiting classification labels Jan 15, 2025
@AndrYurk AndrYurk linked a pull request Jan 17, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature_request New feature request, awaiting triage triage all new issues awaiting classification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant