Skip to content

Add test suite#66

Open
calufa wants to merge 7 commits into
mainfrom
feature/add-unit-test-suite
Open

Add test suite#66
calufa wants to merge 7 commits into
mainfrom
feature/add-unit-test-suite

Conversation

@calufa

@calufa calufa commented Dec 30, 2021

Copy link
Copy Markdown
Contributor

Ticket: #64

TODO:

  • document the commands to run the test suite
  • add coverage reports (i.e. go test -coverprofile)
  • test single functions
  • clean go.mod

Add test suite to:

  • dashboard-server
  • replybot
  • dean
  • linksniffer
  • scribble

@netlify

netlify Bot commented Dec 30, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for vlab-research canceled.

🔨 Explore the source changes: 3d2e20a

🔍 Inspect the deploy log: https://app.netlify.com/sites/vlab-research/deploys/6200a5f6bbee6f0007f427fe

@calufa
calufa force-pushed the feature/add-unit-test-suite branch from 29fc243 to 8321468 Compare December 30, 2021 00:11
@nandanrao

Copy link
Copy Markdown
Contributor

Couple thoughts:

  1. Let's make sure it's documented
  2. Let's make sure that if it doesn't need a database, it doesn't wait on a database (replybot).
  3. Let's make sure we have a way to do coverage reports (i.e. go test -coverprofile) and test single functions (go test -run) etc.

@calufa
calufa force-pushed the feature/add-unit-test-suite branch 9 times, most recently from 718f257 to 37b5e7b Compare January 6, 2022 07:59
@calufa
calufa force-pushed the feature/add-unit-test-suite branch 4 times, most recently from 5caf772 to 4afeb28 Compare January 6, 2022 23:02
@calufa
calufa force-pushed the feature/add-unit-test-suite branch from 4afeb28 to acec833 Compare January 6, 2022 23:34
@calufa
calufa force-pushed the feature/add-unit-test-suite branch from b3273c7 to 0ef81f3 Compare January 6, 2022 23:47
) l USING (userid)`;
) l
USING (userid)
ORDER BY first_timestamp DESC

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is basically a copy and paste from, #59 (comment)

title: 'New User Title',
metadata: '{}',
survey_name: "test",
translation_conf: '{}',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fields required by DB schema.

};
const newUser = await User.create(user);

newUser.token.should.equal(user.token);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like user.token is not implemented?

newUser.email.should.equal(user.email);

user.token = 'HxpnYoykme73Jz1c9DdAxPws77GzH9jLqE1wu1piSqJj';
const userUpdated = await User.update(user);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like we don't have a User.update() method.

};
await User.create(user);
const userFromDb = await User.user(user);
userFromDb.token.should.equal(user.token);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like user doesn't contain token.

Comment thread devops/sql/migrate-21.sql
@@ -0,0 +1,2 @@
ALTER TABLE chatroach.states ADD CONSTRAINT valid_state_json CHECK (state_json ? 'state');

@calufa calufa Jan 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taken from,

CONSTRAINT "valid_state_json" CHECK (state_json ? 'state'),
PRIMARY KEY (userid, pageid)
);`
)

Comment thread devops/sql/migrate-21.sql
@@ -0,0 +1,2 @@
ALTER TABLE chatroach.states ADD CONSTRAINT valid_state_json CHECK (state_json ? 'state');
ALTER TABLE chatroach.states ADD CONSTRAINT fk_facebook_page_id FOREIGN KEY (pageid) REFERENCES chatroach.credentials(facebook_page_id) ON DELETE CASCADE;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Taken from,

pageid VARCHAR NOT NULL NOT NULL REFERENCES credentials(facebook_page_id),

args := m.Called(id, url)
return args.Error(0)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This looks like dead code.

Comment thread scribble/response_test.go
assert.Equal(t, "25", *res[2])
assert.Equal(t, "25", *res[0])
assert.Equal(t, "true", *res[1])
assert.Equal(t, "yes", *res[2])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Strangely, results are returned in a diff order. Is this an ok behavior?

Comment thread scribble/response_test.go

mustExec(t, pool, "drop table responses")
assert.Equal(t, "123", *res[0])
assert.Equal(t, "baz", *res[2])

@calufa calufa Jan 9, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Strangely, results are returned in a diff order. Is this an ok behavior?

Comment thread scribble/response_test.go
assert.Equal(t, "LOL", *res[0])
assert.Equal(t, "A", *res[1])
assert.Equal(t, "A", *res[0])
assert.Equal(t, "LOL", *res[1])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Strangely, results are returned in a diff order. Is this an ok behavior?

@calufa

calufa commented Jan 9, 2022

Copy link
Copy Markdown
Contributor Author

Alright, just double-check the code. Left some comments.

The only services we didn't add test-suites to are:

  • dashboard-client
  • botserver
  • moviehouse

@calufa

calufa commented Jan 9, 2022

Copy link
Copy Markdown
Contributor Author

For test coverage on services written in go, we are now printing the coverage at the beginning of each test-suite run, and it looks something like this:

... 
Running tests...
[nodemon] 2.0.15
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: (all)
[nodemon] starting `go test -cover -tags dynamic`
PASS
coverage: 86.4% of statements <--------------------------

@calufa
calufa requested a review from nandanrao January 9, 2022 03:34
@calufa
calufa marked this pull request as ready for review January 9, 2022 03:34
@calufa
calufa force-pushed the feature/add-unit-test-suite branch from aed15fe to 3a63a22 Compare January 9, 2022 03:39
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.

2 participants