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

HPC-GAP tests print spurious diffs caused by line width differences #136

Closed
fingolfin opened this issue Mar 29, 2015 · 4 comments
Closed
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: HPC-GAP Issues and PRs related to HPC-GAP topic: tests issues or PRs related to tests

Comments

@fingolfin
Copy link
Member

In regular GAP, I can run individual GAP tests in isolation just fine; in particular, the Test function takes care of setting the "virtual" screen width for output of the tests to 80, so that there are no differences caused by my actual window size. For example, I can do this:

gap> Test("tst/grppc.tst");
grppc.tst
GAP4stones: 155136
true
gap> 

In contrast, this mechanism is broken in HPC-GAP. Thus, I get this:

gap> Test("tst/grppc.tst");
########> Diff in tst/grppc.tst, line 65:
# Input is:
c:=11173863408701668767758108041977982393767296210528199906493002694797783834150538286350266065216327192789065778954586535410\
5698690880491419382732505129685548945886493976382779091529311779061982182942409366242406420035526825355893426176;
# Expected output:
111738634087016687677581080419779823937672962105281999064930026947977838341505\
382863502660652163271927890657789545865354105698690880491419382732505129685548\
945886493976382779091529311779061982182942409366242406420035526825355893426176
# But found:
111738634087016687677581080419779823937672962105281999064930026947977838\
341505382863502660652163271927890657789545865354105698690880491419382732\
505129685548945886493976382779091529311779061982182942409366242406420035\
526825355893426176
########
grppc.tst
GAP4stones: 95368
false
gap>

This is quite inconvenient, e.g. when testing regressions that involve manually (re)running tests over and over. I imagine this might be caused by SizeScreen not working as expected in HPC-GAP...

@fingolfin fingolfin added the topic: HPC-GAP Issues and PRs related to HPC-GAP label Mar 29, 2015
@olexandr-konovalov
Copy link
Member

Indeed, the screen width in HPC-GAP slightly differs - @rbehrends might be able to tell more about the plans to fix this. For now, testinstall.g and other tests just use compareFunction:="uptowhitespace" to ignore this, but if one uses Test directly, unfortunately one have to remember to call it as

Test("tst/grpperm.tst",rec(compareFunction:="uptowhitespace")); 

to compare the result only up to whitespaces.

@ChrisJefferson suggests to make uptowhitespace the default behaviour, at lease for now in HPC-GAP, and use exact comparison only when we want to enforce it. That may be a good thing to do.

@stevelinton
Copy link
Contributor

I agree with Chris, but we should also look at the way SizeScreen works in the HPCGAP UI. Should it set the actual size of the screen window, or the line length available for output from a working thread?

@rbehrends
Copy link
Contributor

The issue is the following code in src/scanner.c:

  /* TODO: This should be -2 instead of -8 and room for the prefix
   * accounted for elswhere. */
  else if ( stream->pos < SyNrCols-8-TLS->noSplitLine ) {

In order to make room for the [n] prefix that threads other than the main one use for their output, I turned the -2 into a -8. Ideally, this should be -2-strlen(prefix), but that can be a bit laborious. A quick fix would use something like -2-6*(TLS->threadID != 0) instead of -8 in the above check, restoring the original width for the main thread and making the test work.

@fingolfin fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: tests issues or PRs related to tests labels Dec 16, 2015
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them and removed kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements labels Mar 17, 2017
@fingolfin
Copy link
Member Author

The workaround suggested by @rbehrends was added in a year ago or so. Of course it's just a hack, but we'll just deal with it with a new issue should it resurface eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: HPC-GAP Issues and PRs related to HPC-GAP topic: tests issues or PRs related to tests
Projects
None yet
Development

No branches or pull requests

4 participants