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

Give more helpful warning when using tabs in continuations #3375

Merged
merged 2 commits into from
Mar 25, 2019

Conversation

ChrisJefferson
Copy link
Contributor

This rejects continuations using 'tab' inside tests. They can
be confusing for users because GAP does not treat them as
continuations, but instead a line of output.

They are rejected, rather than treated as continuations, because
we do not want different parts of GAP to handle continuations
differently, or really start changing the definition of something
so fundamental.

This does break any tests which start an output line with '>\t',
but no such tests seem to exist and would be easily confused with
a continuation.

Fixes #3068 (by giving a nice error) while hopefully not breaking any tests (unlike my previous attempt at fixing this, which broke Browse).

@wucas wucas added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them topic: tests issues or PRs related to tests labels Mar 23, 2019
@coveralls
Copy link

coveralls commented Mar 24, 2019

Coverage Status

Coverage decreased (-0.0005%) to 85.137% when pulling 7af058a on ChrisJefferson:test-fix into cd3cc79 on gap-system:master.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Thank you, very useful! Some nitpicking, though

lib/test.gi Outdated Show resolved Hide resolved
tst/testspecial/broken-test-6.tst Show resolved Hide resolved
lib/test.gi Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 25, 2019

Codecov Report

Merging #3375 into master will increase coverage by 0.19%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3375      +/-   ##
==========================================
+ Coverage   85.14%   85.33%   +0.19%     
==========================================
  Files         697      696       -1     
  Lines      344069   342430    -1639     
==========================================
- Hits       292941   292208     -733     
+ Misses      51128    50222     -906
Impacted Files Coverage Δ
lib/test.gi 80.81% <100%> (+0.07%) ⬆️
src/cyclotom.h 50% <0%> (-50%) ⬇️
src/records.h 50% <0%> (-50%) ⬇️
src/gapstate.h 42.85% <0%> (-42.86%) ⬇️
src/code.h 57.14% <0%> (-34.75%) ⬇️
src/gvars.h 66.66% <0%> (-33.34%) ⬇️
src/hookintrprtr.h 66.66% <0%> (-33.34%) ⬇️
src/macfloat.h 57.14% <0%> (-28.58%) ⬇️
src/intobj.h 66.66% <0%> (-27.34%) ⬇️
src/objects.h 65.07% <0%> (-21.49%) ⬇️
... and 83 more

This rejects continuations using 'tab' inside tests. They can
be confusing for users because GAP does not treat them as
continuations, but instead a line of output.

They are rejected, rather than treated as continuations, because
we do not want different parts of GAP to handle continuations
differently, or really start changing the definition of something
so fundamental.

This does break any tests which start an output line with '>\t',
but no such tests seem to exist and would be easily confused with
a continuation.
@ChrisJefferson
Copy link
Contributor Author

All cleaned up. Also, did a quick cleanup using StartsWith consistently.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Nice!

@fingolfin fingolfin merged commit 3bee8d5 into gap-system:master Mar 25, 2019
@markusbaumeister markusbaumeister added the release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes label Apr 15, 2019
@ChrisJefferson ChrisJefferson deleted the test-fix branch May 8, 2019 16:53
@DominikBernhardt DominikBernhardt added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 21, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: unexpected error Issues describing bugs in which computation unexpectedly encounters an error, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them release notes: added PRs introducing changes that have since been mentioned in the release notes topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tabs on continuation lines break GAP's testing
7 participants