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

added contract block to allow truffle-test single file execution #198

Merged
merged 6 commits into from
Aug 15, 2018
Merged

added contract block to allow truffle-test single file execution #198

merged 6 commits into from
Aug 15, 2018

Conversation

mirathewhite
Copy link
Contributor

Added a contract block to unit test files. This will let us run truffle test <filename> and npm run truffle-test -- [file].

Copy link
Contributor

@o-a-hudson o-a-hudson left a comment

Choose a reason for hiding this comment

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

I think this will mean for running all the tests that it will duplicate these tests as it will run in TestWrapper and the individual files

Copy link
Contributor

@o-a-hudson o-a-hudson left a comment

Choose a reason for hiding this comment

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

I think it's a bit confusing to separate the regular and upgraded tests. I think it would be nice to be able to run the test file and have it automatically run both the regular and upgraded tests for that file. What do you think of changing our test structure so that each file the has code you added but instead of running the tests in the file, the function passes run_tests as a parameter to a function in TestWrapper that runs run_tests on both the regular and upgraded tokens? So TestWrapper would only consist of a single function that simply ran whatever test set it was passed on a regular and upgraded token.

@mirathewhite
Copy link
Contributor Author

I like that idea ... I'll update the PR

@mirathewhite
Copy link
Contributor Author

Changes:

  • TestWrapper exposes a single function execute that takes as input a run_tests function and the name of the test suite. execute will execute the run_tests function inside a contract block using a new token and an upgraded token.
  • TestWrapper no longer executes any unit tests independently.
  • Each unit test file calls TestWrapper.execute on its local run_tests function.
  • npm run truffle-test will run all unit tests
  • npm run truffle-test -- [filename] will run all the unit tests in the file on a new token and an upgraded token.

// an upgraded token. The test_suite_name is printed standard output.
function execute(test_suite_name, run_tests_function) {
contract(test_suite_name, async function (accounts) {
await run_tests_function(newUpgradedToken, accounts);
Copy link
Contributor

Choose a reason for hiding this comment

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

Both of these executions seem to be using the newUpgradedToken. Did you intend the first one to use the regular token?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bloops!

Copy link
Contributor

@o-a-hudson o-a-hudson left a comment

Choose a reason for hiding this comment

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

See your change. Could you remove the merge commit (by using a rebase) in this PR? We're trying to avoid including merge commits in PRs going forward.

@o-a-hudson o-a-hudson merged commit 1dd9732 into circlefin:master Aug 15, 2018
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