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

Allow input and output to be mixed in Test #3168

Merged
merged 1 commit into from
Jan 10, 2019

Conversation

ChrisJefferson
Copy link
Contributor

This was previously forbidden as it can lead to confusing errors,
but it also broke some valid Tests, as GAP can begin outputting
before a complete statement has been inputted.

In particular, this previous change broke the "Browse" package.

@codecov
Copy link

codecov bot commented Jan 10, 2019

Codecov Report

Merging #3168 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3168      +/-   ##
==========================================
+ Coverage   83.68%   83.68%   +<.01%     
==========================================
  Files         688      688              
  Lines      336637   336633       -4     
==========================================
- Hits       281724   281721       -3     
+ Misses      54913    54912       -1
Impacted Files Coverage Δ
lib/test.gi 67.66% <ø> (ø) ⬆️
src/stats.c 95.29% <0%> (-0.01%) ⬇️

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.8%) to 74.327% when pulling 85eb95f on ChrisJefferson:test-fix into 8079c16 on gap-system:master.

@coveralls
Copy link

coveralls commented Jan 10, 2019

Coverage Status

Coverage increased (+0.9%) to 83.711% when pulling 32f0a71 on ChrisJefferson:test-fix into ff8cc35 on gap-system:master.

@@ -248,6 +248,25 @@ gap> Unbind(l![fail]);
Error, PosObj Assignment: <position> must be a positive small integer (not the\
value 'fail')

#
# Statements where input + output are mixed
# Checks test can handle output directly cute+pasted from GAP's output
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Checks test can handle output directly cute+pasted from GAP's output
# Checks test can handle output directly cut+pasted from GAP's output

Copy link
Member

Choose a reason for hiding this comment

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

Also, why is this in interpreter.c? It's a test for Test, not the interpreter, isn't it? Note that interpreter.tst is meant to be in sync with coder.tst -- we already violate this right now (my fault) but I was recently working on fixing this again, and would prefer not to drift more.

That said, I'll survive if we add it here, I'll just move it elsewhere when I get to finish that sync I just mentioned.

This was previously forbidden as it can lead to confusing errors,
but it also broke some valid Tests, as GAP can begin outputting
before a complete statement has been inputted.
@fingolfin fingolfin merged commit 5296012 into gap-system:master Jan 10, 2019
@ChrisJefferson ChrisJefferson deleted the test-fix branch January 20, 2019 13:38
@wilfwilson wilfwilson 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
@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 22, 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
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants