Feat/testing language #19

naterush · 2017-10-10T00:09:50Z

add basic testing language that supports control of block generation, sending, safety checks, forkchoice checks, etc.
temp. update function addition to settings (until Create ValidatorSet class #15), so we can change things while running tests easier.
add an explanation of testing to readme.

naterush · 2017-10-10T00:21:03Z

Would like to change reporting (displaying the view graphs) to off by default, but should be able to turn it on from some command line flag.

Not sure how to handle this w/ running unittest discover though (but I haven't tried anything either :) ). Let me know if you have any thoughts - otherwise, I'm going to mess around a bit when I get the chance.

naterush · 2017-10-10T00:23:21Z

Also, worth noting that the tests are wildly incomplete; more tests should be added ASAP so we can better protect ourselves from any big changes (e.g. ValidatorSet).

djrtwo · 2017-10-10T01:00:40Z

settings.py

+
+
+def update(val_weights):
+    global NUM_VALIDATORS


shouldn't have to redeclare these global vars

Think we need to do this to be able to modify existing global variables. See here: https://stackoverflow.com/questions/423379/using-global-variables-in-a-function-other-than-the-one-that-created-them

djrtwo · 2017-10-10T01:07:56Z

Can use https://stackoverflow.com/questions/11380413/python-unittest-passing-arguments/20702984#20702984 for passing in args.

Could use this and configure TestLangCBC to report/not-report depending on something being passed in or not.

naterush · 2017-10-10T02:07:46Z

The issue w/ this is that code (if __name__ == "__main__":) is only run when the test is called directly w/ python -m test.test_SOMETHING.

When it's run from the root directory (which I think we have to do for paths for test to find imports) w/ python -m unittest test.test_SOMETHING, this code doesn't run and so we don't get command line args (at least easily).

djrtwo · 2017-10-10T02:09:27Z

I see. I was a bit skeptical...

djrtwo · 2017-10-10T13:55:02Z

test/test_block.py

+
+        self.assertNotEqual(block_0, block_1)
+
+    def test_non_equality_of_copies_of_non_genesis(self):


@naterush This function name is confusing. Maybe test_non_equality_of_test_lang_blocks.

The point of this function is to make sure that separate blocks created by the test_lang are in fact different blocks, right?

maybe, test_unique_block_creation_in_test_lang

djrtwo · 2017-10-10T13:57:43Z

test/test_block.py

+
+        self.assertFalse(block_0.is_in_blockchain(block_1))
+
+    def test_in_blockchain(self):


test_is_in_blockchain__test_lang

djrtwo · 2017-10-10T13:57:44Z

test/test_block.py

+        block_0 = Block(None, Justification(), 0)
+        block_1 = Block(None, Justification(), 1)
+
+        self.assertFalse(block_0.is_in_blockchain(block_1))


Might call this test_is_in_blockchain__separate_genesis

djrtwo · 2017-10-10T13:58:17Z

test/test_block.py

+        block_0 = Block(None, Justification(), 0)
+        block_1 = Block(None, Justification(), 1)
+
+        self.assertFalse(block_0.is_in_blockchain(block_1))


Run the assertion the other way too, block1.is_in_blockchain(block0)

djrtwo · 2017-10-10T14:37:51Z

test/test_forkchoice.py

+        """
+        test_string = "B1-A S0-A B0-B S1-B S2-A B2-C S1-C H1-C"
+        testLang = TestLangCBC(test_string, [5, 6, 5])
+        with self.assertRaises(AssertionError):


This construct hides what assertion actually fails in the testLang. When I read this test, I'm not actually sure which component of the test_string us supposed to fail, and you as the tester can't be sure where it fails either.

Might be worth having particular Error types that correspond to failing assertions in testLang. For example, if check_head_equals_block fails, you can throw a ForkChoiceError. Then here you can check that a ForkChoiceError was raised, rather than say a SafetyError.

Open to other ways of solving this. Let me know your thoughts. Goal would be to have more transparency as to what exactly you are testing.

Yeah, this is a good point - some reporting here would def be good here, as we don't even know what is causing this test to fail.

Once we move to pytest, we could also likely use the assert statements and 'sys' library to parse the error message that comes with it (see here) - but this is pretty messy. Adding user-defined errors seems like the best way of accomplishing this (and they would only need to exist in the testing world).

@djrtwo Ah, so I misinterpreted a bit what errors we would need here. In this case, this test throws an asserting error b/c of some safety check in the forkchoice itself.

I was thinking that we would add errors to the testing language (which I think is a great idea and what you were suggesting), but do you think we should also add these to the forkchoice as well? Not sure what the best practice is here :~)

Throwing particular errors (ones native to python or our own) allows for us or any developer using this stuff to more easily handle the errors as they please. Assertions should be used for sanity checks and debugging.

This particular example lies somewhere in between. Because we are testing for this and because there are a ton of other assertions currently throughout the codebase, I think we should raise a more specific error.

djrtwo · 2017-10-10T15:13:34Z

test/test_utils.py

@@ -0,0 +1,58 @@
+from testing_language import TestLangCBC


Most of this suite of tests will be one function in py.test with the use of parametrize. woooo

djrtwo

I went through and modded things to generally conform to pep8.
I also changed multi-line test_string declarations to use parenthesis. I think this is nice and readable.
a couple of naming questions.
longer comment about errors raised inside of testLang and how best to distinguish between them

Excellent work. This is a fantastic addition and will make dev way better.

zramsay · 2017-10-10T21:21:04Z

README.md

+## Run Tests
+To run all unit tests:
+```
+python -m unittest discover


this hangs indefinitely w/ no output, and makes a graph with no values.

If you repeatedly kill the graphs as they come up, it moves through the tests. Going to disable plotting for testing by default before merge. Will add option to run plotting as a param when migrate to py.test

@naterush Disable plotting before merge

naterush added 5 commits October 9, 2017 17:22

add testing language

0c47705

add temp update function for tests

5fe420a

add start of basic unit tests

3ffd581

remove profile so tests can run

9b3db15

update the readme for testing (and running simulation)

f158021

naterush requested a review from djrtwo October 10, 2017 00:21

djrtwo reviewed Oct 10, 2017

View reviewed changes

minor test cleanups. conform stuff to pep8

70c5f20

djrtwo reviewed Oct 10, 2017

View reviewed changes

clean up test_safety_oracle strings and pep8

6d4afa7

djrtwo force-pushed the feat/testing_language branch from 1248fba to 6d4afa7 Compare October 10, 2017 15:01

djrtwo reviewed Oct 10, 2017

View reviewed changes

djrtwo added 3 commits October 10, 2017 10:14

conform test_utils.py to pep8

e54e5d5

conform testing_language to pep8

3d9f964

update teststring format in test_forkchoice

29992e0

djrtwo requested changes Oct 10, 2017

View reviewed changes

update test names to be more clear

f7ed7a9

djrtwo mentioned this pull request Oct 10, 2017

circle CI integration #24

Closed

zramsay reviewed Oct 10, 2017

View reviewed changes

turn off view graph display by default

bec1274

djrtwo approved these changes Oct 11, 2017

View reviewed changes

djrtwo merged commit 6c38fef into blockchain Oct 11, 2017

djrtwo deleted the feat/testing_language branch October 11, 2017 11:49

naterush mentioned this pull request Oct 12, 2017

Add user defined errors for easier testing #32

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/testing language #19

Feat/testing language #19

naterush commented Oct 10, 2017 •

edited

naterush commented Oct 10, 2017

naterush commented Oct 10, 2017

djrtwo Oct 10, 2017

naterush Oct 10, 2017

djrtwo commented Oct 10, 2017

naterush commented Oct 10, 2017 •

edited

djrtwo commented Oct 10, 2017

djrtwo Oct 10, 2017

djrtwo Oct 10, 2017 •

edited

djrtwo Oct 10, 2017

djrtwo Oct 10, 2017

djrtwo Oct 10, 2017

djrtwo Oct 10, 2017

naterush Oct 10, 2017 •

edited

naterush Oct 10, 2017

djrtwo Oct 10, 2017

djrtwo Oct 10, 2017

djrtwo left a comment

zramsay Oct 10, 2017

djrtwo Oct 10, 2017 •

edited


		self.assertNotEqual(block_0, block_1)

		def test_non_equality_of_copies_of_non_genesis(self):


		self.assertFalse(block_0.is_in_blockchain(block_1))

		def test_in_blockchain(self):

Feat/testing language #19

Feat/testing language #19

Conversation

naterush commented Oct 10, 2017 • edited

naterush commented Oct 10, 2017

naterush commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djrtwo commented Oct 10, 2017

naterush commented Oct 10, 2017 • edited

djrtwo commented Oct 10, 2017

Choose a reason for hiding this comment

djrtwo Oct 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naterush Oct 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djrtwo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djrtwo Oct 10, 2017 • edited

Choose a reason for hiding this comment

naterush commented Oct 10, 2017 •

edited

naterush commented Oct 10, 2017 •

edited

djrtwo Oct 10, 2017 •

edited

naterush Oct 10, 2017 •

edited

djrtwo Oct 10, 2017 •

edited