Skip to content
This repository was archived by the owner on Sep 25, 2019. It is now read-only.

Conversation

fonglh
Copy link
Member

@fonglh fonglh commented Aug 29, 2016

Originally ASCII-8BIT which causes problems for to_json later in the
Flexirest gem.

Fixes #59, fixes #58.

@fonglh
Copy link
Member Author

fonglh commented Aug 29, 2016

How should I test this?

@fonglh fonglh force-pushed the test-report-encoding branch from e012f00 to f362b37 Compare August 29, 2016 14:54
fonglh added 2 commits August 29, 2016 22:59
Originally ASCII-8BIT which causes problems for `to_json` later in the
Flexirest gem.
@fonglh fonglh force-pushed the test-report-encoding branch from f362b37 to 2420fbb Compare August 29, 2016 15:00
module Coursemology; end
module Coursemology::Evaluator
VERSION = '0.1.5'.freeze
VERSION = '0.1.7'.freeze

Choose a reason for hiding this comment

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

Freezing immutable objects is pointless.

@allenwq
Copy link
Member

allenwq commented Aug 30, 2016

Can you add ASCII-8BIT characters and test ? hard to tell it works or not.

@fonglh
Copy link
Member Author

fonglh commented Aug 30, 2016

It's the same byte sequence, just that the encoding attached to the string is different.

@allenwq
Copy link
Member

allenwq commented Aug 30, 2016

But why not add invalid characters to the template and test ? That will enforce all later changes works with those characters.

@fonglh
Copy link
Member Author

fonglh commented Aug 30, 2016

There are no existing tests that will trigger this bug, even if non ASCII characters are added to the test fixtures.

That's because the exception happens during the .to_json conversion in the flexirest gem, when the save endpoint is called. The existing tests stub save, and the existing tests for the programming evaluation service won't crash even with ASCII-8BIT encoding.

An end-to-end test is needed, but this is actually a test of flexirest, rather than the evaluator gem.

@allenwq allenwq merged commit f3b67b2 into Coursemology:master Aug 30, 2016
@fonglh fonglh deleted the test-report-encoding branch August 30, 2016 11:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Smart quote causes evaluator to crash Bump version to 0.1.7 next time
3 participants