-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Expectations of empty-command-newline #3 are unmeetable (was: Tests XPASS on master) #623
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
Comments
That test has been failing since added a few weeks ago. I assumed you were working in the area. Now that I look at it, shouldn't the third point be |
That test has been failing since added a few weeks ago. I assumed you
were working in the area.
I'm not. I wasn't aware that travis was failing, though.
Now that I look at it, shouldn't the third point be ` '3 3
commandseparator "issue #616"' # \n` rather than `unknown-token` (i.e.
expect the correct result)?
I'm not sure what it should highlight.
I suppose it shouldn't be unknown-token because the sequence semicolon-newline is muscle memory for C and Perl programmers (the rationale behind highlighting BUFFER=': ; ;' as an error doesn't apply here). As to commandseparator, I wonder: should ';' and $'\n' command separators be highlighted differently?
|
The test point is XPASSing, which makes CI red. As a duct tape measure to turn CI green again, update the test expectations to make it XFAIL. The hacky part is that the expectation set by this commit will never be met; the test point will never XPASS now until its expectations are changed again. Issue #623 remains open to track setting the test expectation to the correct value (i.e., make the test XFAIL in a manner that _will_ XPASS if the bug is fixed; in other words, pay off the technical debt created by this commit). Issue #616 remains open to fix the actual bug.
I've added a duct tape fix to the "CI is red" problem and am repurposing this issue to track implementing a proper fix. See the commit message of ab4b6f5 for details. |
Fixed by 8e78e9d. |
make test
is currently failing due to an XPASS:We should change the test file so it either is expected to pass and passes, or is expected to fail and fails.
@phy1729 Were you working on this? What's the correct fix here?
The text was updated successfully, but these errors were encountered: