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

Fix msgread tests #10

Merged
merged 8 commits into from Dec 18, 2015

Conversation

Projects
None yet
2 participants
@paultcochrane
Contributor

paultcochrane commented Jul 5, 2015

The tests in t/25msgread.t were failing and in places incomplete. These changes fix the tests such that they pass and add the test file to the MANIFEST.

paultcochrane added some commits Jul 5, 2015

Extract commented-out debugging code into its own routine
The large block of commented-out debugging code has been extracted into a
new routine which can be enabled by setting the new $debug flag.  This
should make the main test code easier to read while making it also easier to
turn debugging output on and off.
Set expected message text in test data
By using the PDU converter (http://rednaxela.net/pdu.php), one could work
out what the PDU (which was set as simply 'a') should be and thus make some
more of the tests pass.
Add missing character to expected output text
By using the PDU converter: http://rednaxela.net/pdu.php it was possible to
find out why this test wasn't working, and to verify that the generated
output from Device::Gsm was correct.
Remove double text conversion in message read tests
By calling gsm0338_to_iso8859() on the message text was actually converting
a stream of data to ascii which was already encoded as ascii.  The function
gsm0338_to_iso8859() is actually called upon object creation when creating
the `text` field of the Device::Gsm object.
Remove unnecessary, commented-out code
The function called in the commented-out code no longer exists and thus the
commented line can be removed.
Convert msgread tests to Test::More
The test functions in Test::More are more expressive, Test::More is also a
dependency of this module, the code could be simplified a bit, and the
output from a test run is more informative.
@cosimo

This comment has been minimized.

Show comment
Hide comment
@cosimo

cosimo Dec 17, 2015

Owner

OMG. I had never enabled these tests, because they were failing left and right.
I'm not sure I'll remember all the details of the code here to properly review this though :-)
Very nice job, thanks Paul.

Owner

cosimo commented Dec 17, 2015

OMG. I had never enabled these tests, because they were failing left and right.
I'm not sure I'll remember all the details of the code here to properly review this though :-)
Very nice job, thanks Paul.

cosimo added a commit that referenced this pull request Dec 18, 2015

@cosimo cosimo merged commit a326a01 into cosimo:master Dec 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment