-
Notifications
You must be signed in to change notification settings - Fork 5
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 unit-tests for Joda convert. #45
Conversation
b1b32b0
to
02ef0e9
Compare
Alright, I think you can take a look @brunobowden |
c0c4994
to
c9fae08
Compare
Also adds a common script to get test repos from github. Issue j2objc-contrib#30
travis is finally working again - all green @brunobowden |
- TEST_DIR=org.joda-joda-convert TEST_TASKS=build | ||
- TEST_DIR=joda-time-joda-time TEST_TASKS=build | ||
- TEST_DIR=org.joda-joda-primitives TEST_TASKS=build | ||
- TEST_DIR=org.apache.commons-commons-lang3 TEST_TASKS=build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is repeated, it's boiler plate that can be removed ;-)
For now assume that all libraries run the build task for testing. I think that's a good assumption anyway.
If it needs something else in the future, then enumerate the exceptions, not the standard way of operating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd had a plan to allow 2 entries for a TEST_DIR:
- TEST_DIR=com.google.code.gson-gson TEST_TASKS=assemble
- TEST_DIR=com.google.code.gson-gson TEST_TASKS=check
this would allow you to have unit tests in the allowed failures but have translation/compilation in the required successes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either way, i'm removing these changes from this PR as it is only needed in gson, not Joda.
My main question is why offer flexibility on which tasks you run? For now, you're only running "build". Hopefully that should remain the case. Please explain why you want to offer more? |
@brunobowden ptal. Re flexibility:
this would allow you to have unit tests in the allowed failures but have translation/compilation in the required successes. I'll put that in the Gson cl itself though - removed from this PR. |
for task in "${@:2}"; do | ||
./gradlew $task --stacktrace | ||
done | ||
./gradlew assemble |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to do "assemble" before the "build" or other testing tasks. That way, we can more easily distinguish between code that can be used (assemble task) and code that's tested (build task).
LGTM - please ignore my last comments about the assemble / build ordering. I'd do that on the next PR. |
your suggestion is why i had added that code in the first place. unfortunately, it doesn't work as expected because we made a decision long ago that j2objc translation depends on Java unit-testing. we may want to reconsider that. |
Add unit-tests for Joda convert.
Does Travis have a feature where you can mark a certain checkpoint in the build? E.g. "Complete Stages 1&2, Failed at Stage 3"? I searched but couldn't see anything. |
No it doesn't. The only distinction is 'install' vs 'build'. You cannot distinguish between steps of 'build' which is what we'd like |
Also adds a common script to get test repos from github.
Issue #30