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

Add more tests to time_test #4148

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Conversation

datacompboy
Copy link
Contributor

Better test coverage for datetime validation.

Better test coverage for datetime validation.
@grpc-kokoro
Copy link

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.

1 similar comment
@grpc-kokoro
Copy link

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.

@acozzette
Copy link
Member

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

@datacompboy
Copy link
Contributor Author

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

@acozzette
Copy link
Member

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

@datacompboy
Copy link
Contributor Author

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
@datacompboy datacompboy deleted the patch-2 branch January 9, 2018 16:20
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants