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.

Sign up for GitHub

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

Jump to bottom

Integration Test for Job Coordinator #886

Merged
merged 19 commits into from
Jul 21, 2020
Merged

Integration Test for Job Coordinator #886

merged 19 commits into from
Jul 21, 2020

Conversation

pyalex
Copy link
Collaborator

@pyalex pyalex commented Jul 17, 2020 •
edited
Loading

What this PR does / why we need it:

Proposed design principles for integration tests

  1. Avoid mocking. Mocking is based on our expectations how other module works, not its real behavior
  2. Use real dependencies as much as possible via testcontainers
  3. If it's not feasible to use real implementation - implement fully-functional substitution, eg service with in-memory storage, that potentially can be covered with unit tests (see FakeJobManager)
  4. This substitution should be discovered through TestConfiguration
  5. Use smart matchers (hamcrest) instead of building complex object to compare
  6. Use awaitility for handling async processes. Test code should be short

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

woop
woop reviewed Jul 17, 2020
View reviewed changes
core/pom.xml Outdated Show resolved Hide resolved
woop
woop reviewed Jul 17, 2020
View reviewed changes
pom.xml Show resolved Hide resolved
@pyalex pyalex changed the title Integration Tests MVP (Job Coordinator IT) Integration Test for Job Coordinator Jul 17, 2020
@woop woop mentioned this pull request Jul 18, 2020
Merged
@pyalex
Copy link
Collaborator Author

pyalex commented Jul 19, 2020

/test test-end-to-end-batch-dataflow

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 19, 2020

/test test-end-to-end-redis-cluster

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 19, 2020

/test test-end-to-end-batch-dataflow

woop
woop reviewed Jul 20, 2020
View reviewed changes
pom.xml Outdated Show resolved Hide resolved
woop
woop reviewed Jul 20, 2020
View reviewed changes
core/src/test/java/feast/core/it/DataGenerator.java
import java.util.stream.Collectors;
import org.apache.commons.lang3.tuple.Triple;

public class DataGenerator {
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be a generic data generator, because it seems very specific to stores/sources.

Copy link
Collaborator Author

@pyalex pyalex Jul 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Supposed to be generic. Moved fetureSet creation here as well

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 21, 2020

/test test-end-to-end-batch-dataflow

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 21, 2020

/test test-end-to-end-batch-dataflow

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 21, 2020

/test test-end-to-end-batch-dataflow

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 21, 2020

/test test-end-to-end

@pyalex
Copy link
Collaborator Author

pyalex commented Jul 21, 2020

/retest

@woop
Copy link
Member

woop commented Jul 21, 2020

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pyalex, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit c4bcb02 into feast-dev:master Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woop woop woop approved these changes

@davidheryanto davidheryanto Awaiting requested review from davidheryanto

@khorshuheng khorshuheng Awaiting requested review from khorshuheng

@zhilingc zhilingc Awaiting requested review from zhilingc

Assignees

@woop woop

Labels
approved kind/housekeeping lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pyalex @woop @feast-ci-bot @Wirick