Skip to content

Conversation

@zingales
Copy link
Collaborator

No description provided.

@zingales zingales changed the title removing global variables Mock Db test framework option B Sep 17, 2018
@zingales zingales changed the base branch from testframework to addNotev2 September 18, 2018 17:21
@zingales zingales changed the title Mock Db test framework option B [Don't Merge] Mock Db test framework option B Sep 25, 2018
@atmiguel atmiguel changed the base branch from addNotev2 to master October 1, 2018 03:38
@atmiguel atmiguel changed the title [Don't Merge] Mock Db test framework option B Mock Db test framework option B Oct 1, 2018
Copy link
Owner

@atmiguel atmiguel left a comment

Choose a reason for hiding this comment

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

I can't say I agree with putting the database logic into our models. What was the main reason behind that decision?

if err != nil {
panic(err)
}
fmt.Println(bob)
Copy link
Owner

Choose a reason for hiding this comment

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

why are we printing in our tests?

}
}

func assert(tb testing.TB, condition bool, msg string, v ...interface{}) {
Copy link
Owner

Choose a reason for hiding this comment

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

what's tb?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know. As you can see the TB is what the package calls it.

func assert(tb testing.TB, condition bool, msg string, v ...interface{}) {
if !condition {
_, file, line, _ := runtime.Caller(1)
fmt.Printf("\033[31m%s:%d: "+msg+"\033[39m\n\n", append([]interface{}{filepath.Base(file), line}, v...)...)
Copy link
Owner

Choose a reason for hiding this comment

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

what's going on here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this fancyness gets the file and line number of the calling method.

}
}

// equals fails the test if exp is not equal to act.
Copy link
Owner

Choose a reason for hiding this comment

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

this comment would be unnecessary if the variables just had the full names

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i'll remove the comment

// Create testing client
client := &http.Client{}
{
// jar, err := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List})
Copy link
Owner

Choose a reason for hiding this comment

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

do we still need this comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no

"errors"
)

type Category int
Copy link
Owner

Choose a reason for hiding this comment

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

the filename and everything should probably be noteCategory, instead of just category

@@ -0,0 +1,2 @@
CREATE DATABASE cerealnotes_test;
Copy link
Owner

Choose a reason for hiding this comment

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

why do we need a separate test db?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test run against the database, we don't want any bad code to leave extra unexpected data in our database. So for isolation i created a seperate DB.

}

func ClearValuesInTable(db *models.DB, table string) error {
// db.Query() doesn't allow varaibles to replace columns or table names.
Copy link
Owner

Choose a reason for hiding this comment

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

typo in variables

}
}

func ClearValuesInTable(db *models.DB, table string) error {
Copy link
Owner

Choose a reason for hiding this comment

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

why do we need to clear the values in the tables?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to make sure every run of the tests runs on the same input.

}

// equals fails the test if exp is not equal to act.
func equals(tb testing.TB, exp, act interface{}) {
Copy link
Owner

Choose a reason for hiding this comment

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

if we're using these functions everywhere we should put them in a test_util file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

okay we'll just have to be careful about circular dependencies.

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.

3 participants