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 negative tests for chainerx.linalg.* #8371

Merged
merged 2 commits into from
Nov 8, 2019

Conversation

hvy
Copy link
Member

@hvy hvy commented Nov 3, 2019

Avoid using forward_accept_errors and replace them with pytest.raises for the negative tests for the chainerx.linalg routines.

Also added some missing tests and removed seemingly redundant parameterizations.

C.f. #6764 (comment) and #8362.

@hvy hvy added cat:test Test or CI related. ChainerX Related to ChainerX. labels Nov 3, 2019
@emcastillo emcastillo self-assigned this Nov 5, 2019
Copy link
Member

@emcastillo emcastillo left a comment

Choose a reason for hiding this comment

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

LGTM

@emcastillo emcastillo added this to the v7.0.0 milestone Nov 5, 2019
@emcastillo emcastillo added the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Nov 5, 2019
@emcastillo
Copy link
Member

Jenkins, test this please

@emcastillo emcastillo removed the st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. label Nov 5, 2019
@emcastillo
Copy link
Member

waiting to reach consensus on using create_dummy_ndarray

@chainer-ci
Copy link
Member

Jenkins CI test (for commit be8b428, target branch master) failed with status FAILURE.

@hvy
Copy link
Member Author

hvy commented Nov 5, 2019

PTAL.

Copy link
Member

@emcastillo emcastillo left a comment

Choose a reason for hiding this comment

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

LGTM, @toslunar is there anything you want to point out before merging?
🙂

@emcastillo
Copy link
Member

Jenkins, test this please

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 431d984, target branch master) succeeded!

Copy link
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

LGTM, too.

@emcastillo emcastillo merged commit 959eb48 into chainer:master Nov 8, 2019
@hvy hvy deleted the fix-chainerx-linalg-invalid-tests branch November 8, 2019 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:test Test or CI related. ChainerX Related to ChainerX.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants