Robotframework: Consistent handling of whitespace in test data

Created on 15 Mar 2019  路  7Comments  路  Source: robotframework/robotframework

At the moment handling spaces and whitespace in general is not fully consistent. This affects the new parser (#3076) because we need to decide do we want it to have the same quirks or should the behavior be changed.

The description below explains the current behavior, how the new parser works at the moment, and what differences there are. Separate comments go through the benefits and problems these changes would have. You can vote for the changes by adding :+1: or :-1: reaction to them. Written comments are obviously appreciated as well.

Background

Robot Framework splits plain text test data to parts using spaces (two or more) or the pipe character surrounded by spaces.

When we talk about "space", we typically think about the normal ASCII space (\u0020), but there are various other spaces as well. The most common example of other spaces is the no-break space (\u00A0) that looks exactly like the "normal" space but binds words it separates together so that they cannot be wrapped. In addition to be used on purpose, it can be pretty easily be typed accidentally (depends on the OS and the keyboard layout). Other Unicode characters considered spaces are used a lot less frequently and include characters such as en space, thin space and Ogham space mark.

In addition to various spaces, there are also other whitespace characters such as newline and tab. All others except tab are considered line separators, and because Robot first splits data to lines only the tab can be actually seen when processing individual lines.

Finally, there are various Unicode characters that have "space" in their name but aren't actually considered spaces. One example is zero width space.

How Python handles spaces

When dealing with text in Python, methods like str.strip() and str.split() by default consider all characters considered spaces in Unicode to be spaces. For example, ' xxx\u00A0'.strip() produces xxx but 'xxx\N{ZERO WIDTH SPACE}'.strip() doesn't strip anything. The same rule applies also to regular expressions using \s syntax in patterns.

How Robot Framework works now

This explains how Robot Framework 3.1.x and earlier handle spaces in the plain text test data:

  1. Split data to lines.
  2. Convert no-break spaces to normal spaces.
  3. Convert tab characters to two spaces.
  4. Split line to tokens so that the separator is two or more normal spaces. Effectively no-break spaces and even a single tab work as well, but other space characters won't work as a separator. If the line starts with a pipe character, split using pipes surrounded with one or more normal space.
  5. Normalize all spaces within one token into normal spaces using code that is equivalent to token = '聽'.join(token.split()). This both converts all spaces (including en space and Ogham space mark) to "normal" spaces and also collapses spaces so that consecutive spaces are turned into a single space. Collapsing normal spaces only has an effect with the pipe separated format, though, because in the space separated format two or more consecutive spaces would have already been considered to be a separator.

This approach mostly works fine, but there are some strange features:

  • Other space characters than the normal space and the no-break space aren't considered separators but they are eventually converted to normal spaces anyway.
  • Converting spaces (and tabs) loses information. The parsed data cannot be anymore written back to disk as-is.
  • Collapsing spaces is unnecessary. It generally has no effect with the space separated format (it converts, for example, consecutive en spaces to a single space but that's unlikely to matter) and with the pipe separated format preserving spaces could be considered a feature.

Now the new parser currently works

The lexer part of the new parser (#3076) currently splits data using these rules:

  1. Split data to lines.
  2. Split line to tokens so that the separator can be two characters considered spaces (not only normal space) or a single tab character. If the line starts with the pipe character, split from pipes surrounded by one ore more space character (any space).

The basic logic of this approach is the same as with the old parser, but there are some minor differences:

  • Any space character is considered a separator.
  • Spaces and tabs in data tokens aren't converted to normal spaces.
  • Consecutive spaces aren't collapsed. This only affects the pipe separated format.

There are some real benefits compared to the old parser:

  • Simpler.
  • Consistent.
  • Tokens can be written back to the disk without losing information.
  • All kind of spaces can be used in data.
  • In pipe separated format it's possible to have consecutive spaces without escaping.

The main problem of this approach is that, due to the differences explained above, it isn't fully backwards compatible with the old parser. I'll go through the changes in more detail in separate comments and explain both benefits and problems they'd have. At this point it is still possible to change the new parser.

alpha 1 backwards incompatible enhancement medium

Most helpful comment

Spaces and tabs in data tokens aren't converted to normal spaces

The main benefit of this change is not losing information: all tokens could be written back to disk as-is. This is important because we plan to expose the new parser to external editors using the Language Server Protocol.

Another benefit would be being actually able to use these spaces in test data directly. They can be currently used either by using Robot's native escape syntax like \xA0 or \u3000, or by using variables declared elsewhere. In many cases the escape syntax can be more explicit, but in others they can mess up the text where they are used badly.

The main problem with this change is that these characters, especially the no-break space, may have been used in the test data and currently they are normalized to a normal space. This is obviously backwards incompatible, and also so likely to happen that we'd need to deprecate using these characters first. That shouldn't be overly complicated, though, and it's unlikely that these characters are so common that updating test data would be a huge problem either.

Another problem is that separating different spaces can be really challenging. This includes both editing and viewing data, and failure message like Hello world != Hello world where one has a normal space and the other has a no-break space aren't that informative either. The former problem shouldn't be that big, though, because all better editors ought to separate at least the normal space from other spaces, and except to the no-break space other spaces cannot typically be accidentally typed. Same problem also occurs with all Unicode characters that look the same (there are many!).

The problem with different spaces not showing well in error messages is something that can occur regardless how we handle the test data. For example, we could have Hello world with a normal space in our data and we compare it with Hello world with a no-break space returned by the tested system. We should thus try to make it easier to see what these strings contain in general and enhancements like proposed in #2815 help with that.

Although there are real problems with this change, I still believe it's worth it. Parsed tokens containing the same data as on the file system is the bigger improvement, but I wouldn't mind being able to use all kind of (single) spaces in the test data either.

You can vote this change by adding :+1: or :-1: reaction to it. Written comments are obviously appreciated as well.

All 7 comments

Any space character is considered a separator

This change mainly makes the functionality consistent. At the moment it's strange that all spaces cannot be used as a separator but they are nevertheless converted to spaces later. Assuming we want to allow using the no-break space as a separator like nowadays and don't want to convert it to a normal space (see comment below), this change would also ease implementation. It's much easier to use just '\s' (matches any space) in regexps instead of something like '[ \xA0]' (either normal space or no-break space). Similarly we could use just token.strip() and not token.strip(' \xA0').

It's very unlikely this change would cause problems to anyone. That would require someone using consecutive en spaces or other such spaces in data, but because currently they'd be normalized and collapsed into a single normal space it's very unlikely anyone actually has data like this. I consider this change to be safe in a major release without prior deprecation.

You can vote this change by adding :+1: or :-1: reaction to it. Written comments are obviously appreciated as well.

Spaces and tabs in data tokens aren't converted to normal spaces

The main benefit of this change is not losing information: all tokens could be written back to disk as-is. This is important because we plan to expose the new parser to external editors using the Language Server Protocol.

Another benefit would be being actually able to use these spaces in test data directly. They can be currently used either by using Robot's native escape syntax like \xA0 or \u3000, or by using variables declared elsewhere. In many cases the escape syntax can be more explicit, but in others they can mess up the text where they are used badly.

The main problem with this change is that these characters, especially the no-break space, may have been used in the test data and currently they are normalized to a normal space. This is obviously backwards incompatible, and also so likely to happen that we'd need to deprecate using these characters first. That shouldn't be overly complicated, though, and it's unlikely that these characters are so common that updating test data would be a huge problem either.

Another problem is that separating different spaces can be really challenging. This includes both editing and viewing data, and failure message like Hello world != Hello world where one has a normal space and the other has a no-break space aren't that informative either. The former problem shouldn't be that big, though, because all better editors ought to separate at least the normal space from other spaces, and except to the no-break space other spaces cannot typically be accidentally typed. Same problem also occurs with all Unicode characters that look the same (there are many!).

The problem with different spaces not showing well in error messages is something that can occur regardless how we handle the test data. For example, we could have Hello world with a normal space in our data and we compare it with Hello world with a no-break space returned by the tested system. We should thus try to make it easier to see what these strings contain in general and enhancements like proposed in #2815 help with that.

Although there are real problems with this change, I still believe it's worth it. Parsed tokens containing the same data as on the file system is the bigger improvement, but I wouldn't mind being able to use all kind of (single) spaces in the test data either.

You can vote this change by adding :+1: or :-1: reaction to it. Written comments are obviously appreciated as well.

Consecutive spaces aren't collapsed

This change only affects the pipe separated format. It would have the same benefit that no information is lost than not normalizing spaces discussed above. Being able to use consecutive spaces (any spaces, not only normal spaces) without escaping in the pipe separate format could also be useful. Less escaping would, for example, make the pipe separated format easier target for tools that produce Robot Framework data from external models. That would be one new benefit for the pipe separated format that's currently not used that much.

Also this change would be backwards incompatible, but simply because the pipe separate data isn't that widely used, it's unlikely that there's too much data out there that would be affected. Should nevertheless be deprecated first but that ought to be pretty easy.

There are real benefits and problems are pretty small. I think this is worth it.

You can vote this change by adding :+1: or :-1: reaction to it. Written comments are obviously appreciated as well.

I think the problem with backwards incompatibility could be solved by keeping the old parser around, and let the user choose which one to use. There's already an open issue for this: #1283

For example, robot --parser robot:my_parser would import my_parser and use it instead of the built-in parser for .robot files. You could initially set the default to be the existing parser and let users use the new parser via the option. At some later version you can switch the default to the new parser and let users select the old one.

This would make it much easier for the community to enhance the existing parser and possibly submit new parsers, since they can be developed and tested outside of the core. It lets users upgrade to newer versions of robot without having to also upgrade their tests until they are ready to upgrade their tests.

I definitely don't want to keep the old parser around. I don't see much benefits and there would be lot of extra work. Especially testing with both when there are small syntax differences as described in this issue would be annoying.

I'd be fine with a possibility to use custom test data parsers (#1283). Such parsers could produce same robot.parsing objects as Robot's own parser and possibly they could also produce robot.running.TestSuite structure directly. Anyway, I won't have time for that in the foreseeable future.

The more I think about this, the better the way the new parser currently handles whitespace feels. I've also informed our users about these changes on Slack, Twitter, LinkedIn and robotframework-users and at least so far nobody has objected.

The decision is to keep the current design of the new parser. Things to do:

  • [x] Submit issues deprecating the current behavior in RF 3.1.2.
  • [x] Add unit tests for the new parser to validate that spaces aren't normalized.
  • [x] Fix acceptance tests.
  • [x] ~Update documentation in the User Guide~. This is covered by #3293 along with documenting other parsing changes.

This ought to be done once the new parser branch is merged.

Was this page helpful?
0 / 5 - 0 ratings