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.

Sign up for GitHub

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

Jump to bottom

Add Windows CI jobs #7433

Closed
wants to merge 17 commits into from
Closed

Add Windows CI jobs #7433

wants to merge 17 commits into from

Conversation

disktnk
Copy link
Member

@disktnk disktnk commented Jun 7, 2019 •
edited

  • only python 3.7 and cuda 10.0
  • on cpu and gpu
  • need to fix tests\chainer_tests\test_init_docstring.py fail

@niboshi niboshi self-assigned this Jun 7, 2019
@niboshi
Copy link
Member

niboshi commented Jun 7, 2019

How can we test the PR?

@niboshi niboshi added cat:test Test or CI related. to-be-backported Pull request that should be backported. labels Jun 7, 2019
@disktnk
Copy link
Member Author

disktnk commented Jun 7, 2019

chainer.win.{gpu,cpu} project has already created on pfnci, so add these project to webhook and write comment like "test please", then the jobs will start. But I don't know a correct way to add webhook on this project so have not added webhook yet. CC: @kmaehashi

@niboshi niboshi self-requested a review June 7, 2019 09:19
@niboshi
Copy link
Member

niboshi commented Jun 10, 2019

Jenkins, test this please

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 72e6c2f, target branch master) succeeded!

@niboshi niboshi added the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Jun 12, 2019
niboshi
niboshi requested changes Jun 13, 2019
View reviewed changes
Copy link
Member

@niboshi niboshi left a comment

Choose a reason for hiding this comment

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

Please check the review.
Also, can I expect that the tests\chainer_tests\test_init_docstring.py issue will be solved in this PR?

.pfnci/install_cupy.bat
@@ -0,0 +1,23 @@
@echo off

set CHAINER_LATEST_MAJOR_VER=7
Copy link
Member

Choose a reason for hiding this comment

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

Should we update the version number on each major release?
How about other approach?
(What I come up with immediately is to check presence of the branch v%CHAINER_MAJOR_VER% after clone).

.pfnci/install_cupy.bat Outdated
pip install cython
pip install -e . -vvv

cd %TEST_HOME%
Copy link
Member

Choose a reason for hiding this comment

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

How about pushd/popd?
Or, I don't think cd is necessary (pip install -e cupy/ -vvv).

Copy link
Member Author

Choose a reason for hiding this comment

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

pushd / popd is good idea, thx

I think that installing cupy or other depend module on top repo directory is unsafe and should avoid.

.pfnci/run_test.bat Outdated

set PYTEST_ATTR=not slow and not ideep

curl -o xpytest.exe --insecure -L https://github.com/disktnk/xpytest/releases/download/v0.0.1/xpytest.exe
Copy link
Member

Choose a reason for hiding this comment

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

This line looks very insecure actually...

@disktnk
Copy link
Member Author

disktnk commented Jun 13, 2019

thank you for your revies

can I expect that the tests\chainer_tests\test_init_docstring.py issue will be solved in this PR?

I don't have a plan to fix it.

@niboshi
Copy link
Member

niboshi commented Jun 14, 2019

can I expect that the tests\chainer_tests\test_init_docstring.py issue will be solved in this PR?

I don't have a plan to fix it.

It seems this error is only occurring in this PR.
I think I can't merge the PR unless it's fixed.

@disktnk
Copy link
Member Author

disktnk commented Jun 14, 2019

It seems this error is only occurring in this PR.

I thought the error causes only Windows env.

I'd like to know how to stop the error on xpytest? CC: @imos @kmaehashi

@disktnk
Copy link
Member Author

disktnk commented Jun 15, 2019

Ah, "`I thought the error causes only Windows env." it true, (misunderstanding is misunderstanding). I could reproduce the error without xpytest nor this PR, I reported an issue #7523

As you said, this PR cannot merge until the error is resolve, I'll wait it. @niboshi

@niboshi niboshi removed the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Jun 17, 2019
@toslunar toslunar mentioned this pull request Jul 5, 2019
Merged
@niboshi niboshi added the st:blocked-by-another-pr State indicating that another ticket is preventing this ticket from being closed/merged. label Jul 16, 2019
@stale
Copy link

stale bot commented Oct 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Not updated for a longer period of time. label Oct 14, 2019
toslunar
toslunar reviewed Oct 17, 2019
View reviewed changes
.pfnci/run_test.bat
rem protobuf==3.7.1 with python3.7 causes "DeprecationWarning"
rem to avoid the warning, install specific version
rem see https://github.com/chainer/chainer/issues/7523
pip install protobuf==3.6.1
Copy link
Member

Choose a reason for hiding this comment

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

The workaround could be removed because #7529 is merged and a newer protobuf is available.

@stale stale bot removed the stale Not updated for a longer period of time. label Oct 17, 2019
@disktnk
Copy link
Member Author

disktnk commented Oct 17, 2019

sorry for unattended, tests on windows os are failed, issued #7768. If need some time to fix the issue, CI can skip the target test.

@stale
Copy link

stale bot commented Jan 15, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Not updated for a longer period of time. label Jan 15, 2020
@niboshi niboshi added this to the v7.3.0 milestone Feb 4, 2020
@stale stale bot removed the stale Not updated for a longer period of time. label Feb 4, 2020
@kmaehashi
Copy link
Member

Blocked by #8465

@kmaehashi kmaehashi assigned kmaehashi and unassigned niboshi Mar 31, 2020
@emcastillo emcastillo modified the milestones: v7.4.0, v7.6.0 Apr 23, 2020
@emcastillo
Copy link
Member

#8465 was merged

@emcastillo emcastillo removed this from the v7.7.0 milestone Jul 30, 2020
@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Not updated for a longer period of time. label Dec 25, 2020
@stale
Copy link

stale bot commented Jun 26, 2021

This issue is closed as announced. Feel free to re-open it if needed.

@stale stale bot closed this Jun 26, 2021
@kmaehashi kmaehashi added this to the Closed issues and PRs milestone Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toslunar toslunar toslunar left review comments

@niboshi niboshi niboshi requested changes

Assignees

@kmaehashi kmaehashi

Labels
cat:test Test or CI related. st:blocked-by-another-pr State indicating that another ticket is preventing this ticket from being closed/merged. stale Not updated for a longer period of time. to-be-backported Pull request that should be backported.
Projects
None yet
Milestone
Closed issues and PRs
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@disktnk @niboshi @chainer-ci @kmaehashi @emcastillo @toslunar