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

Cram tests #34

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

Cram tests #34

wants to merge 25 commits into from

Conversation

alexispurslane
Copy link

This isn't perfectly ideal, and I'm not quite done with the tests yet, but it's better than nothing!

@tbodt tbodt mentioned this pull request Dec 16, 2014
@nickserv
Copy link
Contributor

👍 This looks pretty cool, I haven't seen cram before. It would probably help to add a simple make task that runs the cram tests though.

Also, we could probably clean up the syntax a little by using something like here strings and by somehow either adding streem to the user's path or some cram-specific path before the tests are run (or you can just alias streem). I'm not sure that would be possible though, since I haven't used cram myself.

Example

Streem should recognise a stream expression:
  $ streem <<< "STDIN | STDOUT"
  Syntax OK

@mattn
Copy link
Contributor

mattn commented Dec 18, 2014

I don't like adding dependency in this phase. And I guess that cram doesn't seems to work on windows.

@nickserv
Copy link
Contributor

I think cram could be useful, especially if our tests get a little more complicated. Also, we'll probably want to move to a fancier testing tool eventually.

However, we could write our own simple test tool. For example, we could have a script that opens a file with a snippet on each line, inputs each line into streem, and prints out something when there's an error.

@mattn
Copy link
Contributor

mattn commented Dec 18, 2014

Yes, I prefer to make owr test tool.

$ ls t/*.t | bin/streem -e 'STDIN | {|x| system("bin/streem " + x) }'

@nickserv
Copy link
Contributor

I just made a very simple test script using @ChristopherDumas's snippets: https://github.com/nicolasmccurdy/streem/blob/test-snippets/test/test_snippets.sh

However, it's a work in progress, and I should probably hide successes and add some sort of useful status for make and other testing/CI tools.

@alexispurslane
Copy link
Author

Cram should work on windows, as long as you have python and pip installed.

@alexispurslane
Copy link
Author

I will work on making the tests cleaner like you suggested, @nicolasmccurdy. I will also add tests. If possible, can someone list the features of stream so far so that I can make tests for them?

@mattn
Copy link
Contributor

mattn commented Dec 18, 2014

Windows doesn't support <<< in nicolasmccurdy's comment at least. #34 (comment)

@nickserv
Copy link
Contributor

I didn't notice that, thanks. echo "STDIN | STDOUT" | streem should work instead (assuming that Windows has pipe and echo support).

@alexispurslane
Copy link
Author

So are we going to merge this into the main streem, or should I close this??

@mattn
Copy link
Contributor

mattn commented Dec 19, 2014

Not hurry. We know you want to contribute. But this should do carefully considered, I think.

@alexispurslane
Copy link
Author

I'll write more tests in the meantime.

@tbodt
Copy link

tbodt commented Dec 21, 2014

Github's Linguist thinks the test files are Perl files, because the extension is .t. I think it should be changed to .test.

@alexispurslane
Copy link
Author

Ok, changeing.....

@halogenandtoast
Copy link
Contributor

Any reason not to write the tests in C until they can be self-hosted?

@alexispurslane
Copy link
Author

Becouse that would be sort of hard and would result in scary test code that no one would want to change, so we would end up here anyway.

@nickserv
Copy link
Contributor

Wow, these tests are long... Is that just because they're so detailed, or is there old "good" output of the parser pasted into the tests?

@alexispurslane
Copy link
Author

What cram does is it matches the output of the command with it's expected output. That is the output of the parser when run on code. It is long, I agree.

@nickserv
Copy link
Contributor

It seems to me that the current tests are either too low level (and too brittle), or too based on existing output of an older version of the code (which is assuming that version has 0 bugs). That being said, cram looks pretty cool, I really liked the older versions of your tests, and I'm not against tests. I'm just not sure if we want something this complicated and brittle, which is based on trusting an older version of what the code did instead of what a human thinks it should be doing directly.

@alexispurslane
Copy link
Author

OK. I'll work on using an updated version of streem and making the tests less brittle.

@@ -0,0 +1,4962 @@
Streem should recognise the sequence funciton:
Copy link
Contributor

Choose a reason for hiding this comment

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

function*

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.

9 participants