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

fix a regression in Test #5553

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

ThomasBreuer
Copy link
Contributor

After reporting a difference,
reset the line length to the previous value not to the default value.

resolves #5552

After reporting a difference,
reset the line length to the previous value not to the default value.
@ThomasBreuer ThomasBreuer added topic: tests issues or PRs related to tests release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Jan 3, 2024
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.

Fine by me. Would be good if @ChrisJefferson had another look, though.

@fingolfin
Copy link
Member

Actually there is also PR #5549 which seems to be about the same issues, more or less, but a bit differently. (I asked @ChrisJefferson to review that, too, some time ago...). It would be good if you three (@ThomasBreuer @zickgraf @ChrisJefferson could figure out which way to go...)

@ThomasBreuer
Copy link
Contributor Author

Thanks @fingolfin .
As far as I see, #5549 and #5553 do the same:

@zickgraf
Copy link
Contributor

zickgraf commented Jan 4, 2024

I also think that both PRs do the same. Since I was not fully convinced by the variable name current_size I used in #5549, I'm totally fine with using #5553 :-)

@ThomasBreuer
Copy link
Contributor Author

Thanks @zickgraf.
I have now added a commit to remove the unnecessary local variable testsize; all we need is to set the linelength once before starting the tests, and later to switch between the linelengths outside the tests and inside the tests.
@ChrisJefferson ?

@ChrisJefferson
Copy link
Contributor

Looks good to me, I'll just let the tests all cycle, then I'm happy to merge (or someone else can hit the button before me)

@ThomasBreuer ThomasBreuer merged commit 6008cca into gap-system:master Jan 4, 2024
22 checks passed
@ThomasBreuer ThomasBreuer deleted the TB_SizeScreen_Test branch January 4, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: tests issues or PRs related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strange behaviour of Test in the master branch
4 participants