Skip to content

Unit test for java.time #312

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

Merged
merged 16 commits into from
Jan 24, 2022
Merged

Unit test for java.time #312

merged 16 commits into from
Jan 24, 2022

Conversation

gaurangkudale
Copy link
Contributor

Added the unit test for java.time

@gaurangkudale
Copy link
Contributor Author

gaurangkudale commented Jan 20, 2022

Hi @cyrille-artho,
As you said I've done the changes and created the new pull request please give me your thought on this and I want to work on any javapathfinder project if you have any project available for me please let me know.
I'm really excited and looking forward to do more contribution in JPF and learn new things

@gaurangkudale
Copy link
Contributor Author

@cyrille-artho
Any updates?

@gaurangkudale
Copy link
Contributor Author

Hi @cyrille-artho,
Here is a quick update I've added some more easy and simple test cases where java.time package is used waiting for your review

Copy link
Member

@cyrille-artho cyrille-artho left a comment

Choose a reason for hiding this comment

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

Thank you for the new tests. They are very good overall, and I think they cover the functionality sufficiently. As long as no native code is invoked, the existing library functions can be used without problems.

There is one place where an assertion is repeated (lines 20, 21); please remove the duplicate. Other than that, the tests look good, and we can include them after that change.

@gaurangkudale
Copy link
Contributor Author

gaurangkudale commented Jan 21, 2022

Hi @cyrille-artho
Changes are done and there is one more thing I want to work on any javapathfinder core project if you have any project available for me please let me know.
I'm really excited and looking forward to do more contributions to JPF and learning new things
Working on a new project will help me to expand my knowledge

@cyrille-artho
Copy link
Member

Thanks for your changes. A final question: What do you mean with

//TODO show time difference is either 0 or 60`

If you subtract one timestamp from an identical one, shouldn't the difference always be 0? Unless I'm wrong, the comment can be removed.

@cyrille-artho
Copy link
Member

As of new tasks, the most urgent tasks are to ensure that all key features in Java 11 work as they did in Java 8.
There are a couple of unit tests that fail on Java 11, and sometimes, a smaller test can help us narrow down the root cause and develop a fix.

@gaurangkudale
Copy link
Contributor Author

tract one timesta

Yeah that's Right I'LL remove that comment

@cyrille-artho
Copy link
Member

One test fails now:

 java8.ClockTest > switch_clock_time_zone_test FAILED
    java.lang.AssertionError at ClockTest.java:32

You can check the CI build status to get this information, by following the "Details" link next to the red X that marks the failed build.

@gaurangkudale
Copy link
Contributor Author

Hi@cyrille-artho
can you please review the code now?

@gaurangkudale
Copy link
Contributor Author

One test fails now:

 java8.ClockTest > switch_clock_time_zone_test FAILED
    java.lang.AssertionError at ClockTest.java:32

You can check the CI build status to get this information, by following the "Details" link next to the red X that marks the failed build.

Actually I'm not familiar with CI
I want to learn CI/CD from can you please guide me from where should I start

@cyrille-artho
Copy link
Member

Great, it works now!
You can find a good short intro about CI on YouTube: https://www.youtube.com/watch?v=1er2cjUq1UI

@cyrille-artho cyrille-artho merged commit 3fa74ab into javapathfinder:master Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants