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

Remove redundant tests #466

Merged
merged 5 commits into from
Nov 21, 2019
Merged

Remove redundant tests #466

merged 5 commits into from
Nov 21, 2019

Conversation

gusmith
Copy link
Contributor

@gusmith gusmith commented Nov 19, 2019

This PR will close #457
Currently, the entity-service is trying to do unit tests in an end to end way. For example, the removed tests were about testing the run endpoint by creating full runs.
These full runs are already created in a number of other tests (e.g. the ones where we check the validity of the output of a run).
So in this PR, I removed some tests about intermediary results such as checking that a run is progressing normally and testing the run endpoint.

However, this is slightly against the tests visions where each test describe what it is "unit" testing. In the approach I started, we run the whole end to end pipeline and on the way test each part.

But from 327 tests running in 7m 40s on my machine with a simple local deployment on the service, the PR reduced the number of tests to 283 ran in 6m 35s with the same deployment.

The fact that a run is progressing is tested by running the whole run.
…ogressing validly.

Thus remove some unused utility methods.
@gusmith gusmith requested a review from wilko77 November 19, 2019 06:21
@gusmith gusmith self-assigned this Nov 19, 2019
@@ -362,7 +350,7 @@ def from_string(state):
return State.completed


def has_not_progressed_invalidly(status_old, status_new):
def has_progressed_validly(status_old, status_new):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not not approve of this change ;)

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.

tests test_project_run_status are currently useless (or redundant)
2 participants