Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

Add e2e test #86

Merged
merged 13 commits into from Jan 30, 2020
Merged

Add e2e test #86

merged 13 commits into from Jan 30, 2020

Conversation

dabonnie
Copy link
Contributor

Adds an e2e test to launch and verify functionality.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Fix linting errors

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@codecov
Copy link

codecov bot commented Jan 29, 2020

Codecov Report

Merging #86 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   39.44%   39.44%           
=======================================
  Files          33       33           
  Lines        1374     1374           
  Branches      822      822           
=======================================
  Hits          542      542           
  Misses         65       65           
  Partials      767      767
Flag Coverage Δ
#unittests 39.44% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bf4e92...0b156b9. Read the comment docs.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie dabonnie marked this pull request as ready for review January 29, 2020 21:42
@piraka9011
Copy link
Contributor

Does codecov consider this e2e test at all?

test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
@piraka9011
Copy link
Contributor

The tests currently fail immediately by raising an exception. Do we want to still run the rest of the tests even if one fails.

Copy link
Member

@thomas-moulard thomas-moulard left a comment

Choose a reason for hiding this comment

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

Thank you very much Devin for taking the time to write this e2e test, this will immensely useful.

Could you have a pass on my comments? I'd like to secure it to make sure it does not become flaky, and a maintenance burden to the team. Generally, the structure and the idea LGTM (it could probably even be generalized and used in multiple projects!)

test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
@thomas-moulard
Copy link
Member

A link on why negative if-s are not prefered: https://schneide.blog/2014/08/03/dont-ever-not-avoid-negative-logic/ :)

@dabonnie
Copy link
Contributor Author

The tests currently fail immediately by raising an exception. Do we want to still run the rest of the tests even if one fails.

The tests are structured in order to fast fail. For example, if a node doesn't enumerate or if the lifecycle nodes are inactive, then the publisher test will ultimately fail.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie
Copy link
Contributor Author

Note for the reviewers: there are a few unresolved comments that I am still implementing.

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Add topic enumeration test

Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie
Copy link
Contributor Author

Note for the reviewers: there are a few unresolved comments that I am still implementing.

This is ready for re-review.

Copy link
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

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

It looks you addressed most of the outstanding comments offline so LGTM :shipit:

test/run_e2e_test.py Outdated Show resolved Hide resolved
test/run_e2e_test.py Outdated Show resolved Hide resolved
Signed-off-by: Devin Bonnie <dbbonnie@amazon.com>
@dabonnie dabonnie merged commit beda3ad into master Jan 30, 2020
@dabonnie dabonnie deleted the dabonnie/e2e-test branch January 30, 2020 22:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants