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

Skip test involving fork() on windows #3507

Merged
merged 1 commit into from
Jun 23, 2019

Conversation

fingolfin
Copy link
Member

This otherwise is far too slow on 64 bit windows.

Fixes #3506

@fingolfin fingolfin added os: windows Issues and PRs that are (at least partially) specific to Windows topic: tests issues or PRs related to tests backport-to-4.10 labels Jun 18, 2019
@olexandr-konovalov
Copy link
Member

Thanks, but this is not enough. As #3506 says, even the first iteration hangs it. You need also to tweak parameters in

children := List([1..20], x -> runChild(Random([1..2000]), Random([false,true])));;

What worked for me was simple runChild(1,false) and runChild(1,true).

@coveralls
Copy link

coveralls commented Jun 18, 2019

Coverage Status

Coverage remained the same at 85.26% when pulling 6967c23 on fingolfin:mh/children into f7f36d7 on gap-system:master.

@olexandr-konovalov olexandr-konovalov added the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Jun 18, 2019
This otherwise is far too slow on 64 bit windows.

Fixes gap-system#3506
@fingolfin fingolfin changed the title Reduce iterations of test involving fork on windows Skip test involving fork() on windows Jun 19, 2019
@codecov
Copy link

codecov bot commented Jun 19, 2019

Codecov Report

Merging #3507 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3507      +/-   ##
==========================================
+ Coverage   85.42%   85.43%   +0.01%     
==========================================
  Files         699      699              
  Lines      346778   346778              
==========================================
+ Hits       296229   296270      +41     
+ Misses      50549    50508      -41
Impacted Files Coverage Δ
src/iostream.c 65.15% <0%> (-0.76%) ⬇️
src/stats.c 94.97% <0%> (-0.21%) ⬇️
src/opers.c 94.87% <0%> (+0.06%) ⬆️
src/listfunc.c 97.47% <0%> (+0.19%) ⬆️
src/weakptr.c 97.96% <0%> (+0.4%) ⬆️
src/stringobj.c 94.38% <0%> (+0.59%) ⬆️
src/streams.c 76.62% <0%> (+1.39%) ⬆️
src/sysfiles.c 41.39% <0%> (+1.86%) ⬆️

@fingolfin
Copy link
Member Author

I've decided to simply don't run this part of the test file at all on Windows. It's IMHO just not worth the hassle to try to just launch 1-2 child processes: we already have other tests which do that, so we don't gain anything from it.

@olexandr-konovalov
Copy link
Member

Ok! I guess I still can test #3513 where this test is not modified?

@fingolfin
Copy link
Member Author

@alex-konovalov does your approval of this PR mean that you tested it on Windows / Cygwin and it resolved the issue there?

@olexandr-konovalov
Copy link
Member

No - it's pretty obvious that this test should be excluded from Windows, so my approval is just based on inspecting the diff.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.10.3 milestone Jun 21, 2019
@fingolfin
Copy link
Member Author

Argh, I confused the PRs, my bad :-(

@olexandr-konovalov olexandr-konovalov removed the do not merge PRs which are not yet ready to be merged (e.g. submitted for discussion, or test results) label Jun 23, 2019
@olexandr-konovalov olexandr-konovalov merged commit 597c793 into gap-system:master Jun 23, 2019
@olexandr-konovalov
Copy link
Member

Backported to stable-4.10 in 2dbf56e

@fingolfin fingolfin deleted the mh/children branch June 24, 2019 21:18
@fingolfin fingolfin added the release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes label Aug 21, 2019
@olexandr-konovalov olexandr-konovalov modified the milestones: GAP 4.10.3, GAP 4.11.0 Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE os: windows Issues and PRs that are (at least partially) specific to Windows 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.

teststandard/processes/children.tst unusable in 64-bit Windows build
3 participants