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

Add more tests to time_test #4148

Merged
merged 1 commit into from Jan 8, 2018

Conversation

Projects
None yet
5 participants
@datacompboy
Contributor

datacompboy commented Jan 5, 2018

Better test coverage for datetime validation.

Update time_test.cc
Better test coverage for datetime validation.
@grpc-jenkins

This comment has been minimized.

Show comment
Hide comment
@grpc-jenkins

grpc-jenkins Jan 5, 2018

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

grpc-jenkins commented Jan 5, 2018

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@grpc-jenkins

This comment has been minimized.

Show comment
Hide comment
@grpc-jenkins

grpc-jenkins Jan 5, 2018

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

grpc-jenkins commented Jan 5, 2018

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@googlebot googlebot added the cla: yes label Jan 5, 2018

@acozzette

This comment has been minimized.

Show comment
Hide comment
@acozzette

acozzette Jan 5, 2018

Contributor

ok to test (this comment will trigger the other test runs)

Contributor

acozzette commented Jan 5, 2018

ok to test (this comment will trigger the other test runs)

@datacompboy

This comment has been minimized.

Show comment
Hide comment
@datacompboy

datacompboy Jan 5, 2018

Contributor

This will cause tests to fail (intentently), as it will succeed only after pull/4147 merged.

Contributor

datacompboy commented Jan 5, 2018

This will cause tests to fail (intentently), as it will succeed only after pull/4147 merged.

@acozzette

This comment has been minimized.

Show comment
Hide comment
@acozzette

acozzette Jan 5, 2018

Contributor

Ok, I figured that might be the case, I'll just merge it as long as that's the only test failure.

Contributor

acozzette commented Jan 5, 2018

Ok, I figured that might be the case, I'll just merge it as long as that's the only test failure.

@datacompboy

This comment has been minimized.

Show comment
Hide comment
@datacompboy

datacompboy Jan 5, 2018

Contributor

We see here failure (intended);
[ RUN ] DateTimeTest.WrongDays
Assertion failed: time.day >= 1 && time.day <= (month == 2 && IsLeapYear(year) ? kDaysInMonth[month] + 1 : kDaysInMonth[month]), file C:\projects\protobuf\src\google\protobuf\stubs\time.cc, line 123

It worth re-trigger update after fix merged, to make sure tests get fixed.

Contributor

datacompboy commented Jan 5, 2018

We see here failure (intended);
[ RUN ] DateTimeTest.WrongDays
Assertion failed: time.day >= 1 && time.day <= (month == 2 && IsLeapYear(year) ? kDaysInMonth[month] + 1 : kDaysInMonth[month]), file C:\projects\protobuf\src\google\protobuf\stubs\time.cc, line 123

It worth re-trigger update after fix merged, to make sure tests get fixed.

@acozzette acozzette merged commit b77aa80 into protocolbuffers:master Jan 8, 2018

1 of 5 checks passed

Jenkins Build finished.
Details
Jenkins 32bit Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
cla/google All necessary CLAs are signed

@datacompboy datacompboy deleted the datacompboy:patch-2 branch Jan 9, 2018

@xfxyjwf xfxyjwf added the c++ label Jun 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment