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

Fixing tests with time.Now #1539

Merged
merged 4 commits into from Jan 2, 2019

Conversation

4 participants
@akostibas
Copy link
Contributor

akostibas commented Jan 2, 2019

Description

There were a couple things breaking server_test:

  1. A test was using time.Now, which meant that the data it was creating slowly was leaving the window where other test data (performance period data, to be specific) existed to support it. Jan 1 was the cut off, and then the test started failing. I changed the test to use the Test Data Gen constants.
  2. make server_test was failing because there was an order of operations problem in the make target. The test DB was getting created, then destroyed. It should be destroyed, then created.

@akostibas akostibas requested review from jim , chrisgilmerproj and jacquelineIO Jan 2, 2019

@stangah

stangah approved these changes Jan 2, 2019

Copy link
Contributor

stangah left a comment

lgtm, though fix make target plz

Makefile Outdated
@@ -178,7 +178,7 @@ else
go test -p 1 -count 1 -short $$(go list ./... | grep \\/cmd\\/webserver) 2> /dev/null
endif

server_test: server_deps server_generate db_test_run db_test_reset db_test_migrate
server_test: server_deps server_generate db_test_reset db_test_run db_test_migrate

This comment has been minimized.

@stangah

stangah Jan 2, 2019

Contributor

db_test_reset calls db_test_run, so I think you can just remove it

This comment has been minimized.

@chrisgilmerproj

chrisgilmerproj Jan 2, 2019

Contributor

Yeah, no reason to add db_test_run here.

@jacquelineIO
Copy link
Contributor

jacquelineIO left a comment

:shipit: lgtm

@jacquelineIO

This comment has been minimized.

Copy link
Contributor

jacquelineIO commented Jan 2, 2019

Fixes the current issue, but when TestYear changes to 2019 it may break again until we fix the rateengine testdata. Will there be a new story to cover this?

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

One fix to the Makefile and you'll be ready to go.

akostibas added some commits Jan 2, 2019

@akostibas

This comment has been minimized.

Copy link
Contributor

akostibas commented Jan 2, 2019

Good call @jacquelineIO. Made a story for the BatTeam here: https://www.pivotaltracker.com/story/show/162932482

@akostibas akostibas merged commit 7a1637c into master Jan 2, 2019

12 checks passed

ci/circleci: acceptance_tests Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details

@akostibas akostibas deleted the ak-fixing-tests-with-now branch Jan 2, 2019

@jacquelineIO jacquelineIO referenced this pull request Jan 3, 2019

Merged

Adding test for and fixing bug with payment for ppm #1551

0 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment