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

AVRO-1887 Integrate Yetus and fix tests #353

Merged
merged 2 commits into from
Nov 7, 2018
Merged

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Oct 24, 2018

All,

First PR that will kick off the tests using Apache Yetus. It is a crude implementation since it will only kick off the ./build.sh, but this will be refined later on.

The C++ and the interop tests are failing and are therefore disabled for now.

cc @kojiromike

Cheers, Fokko

@Fokko Fokko requested a review from cutting October 24, 2018 09:45
@Fokko Fokko force-pushed the fd-fix-tests branch 6 times, most recently from c28c31e to a5a21e2 Compare October 28, 2018 22:28
Copy link
Contributor

@kojiromike kojiromike left a comment

Choose a reason for hiding this comment

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

I don't know Yetus at all, but I'm assuming it'll give us some useful feedback in github checks. I have some suggestions for bash script improvements. Can Yetus be configured to run shellcheck on them?

@Fokko Fokko force-pushed the fd-fix-tests branch 2 times, most recently from c2916d8 to 5cc2e4f Compare October 29, 2018 14:59
@Fokko
Copy link
Contributor Author

Fokko commented Oct 29, 2018

Thanks for the review @kojiromike

Enabling shellcheck is easy by adding it in the list of plugins: https://github.com/Fokko/avro/blob/fd-fix-tests/.travis.yml#L34

There is also support for hadolint etc.

@Fokko Fokko force-pushed the fd-fix-tests branch 5 times, most recently from 44e5865 to 4fdcb14 Compare October 29, 2018 18:57
@Fokko Fokko mentioned this pull request Oct 29, 2018
@Fokko Fokko force-pushed the fd-fix-tests branch 8 times, most recently from 10e1c2a to 778e9e2 Compare November 4, 2018 23:37
@Fokko Fokko changed the title AVRO-1887 Integrate Yetus [WIP] AVRO-1887 Integrate Yetus Nov 5, 2018
@Fokko Fokko force-pushed the fd-fix-tests branch 4 times, most recently from 77faaf5 to 057636a Compare November 5, 2018 14:34
@Fokko Fokko changed the title [WIP] AVRO-1887 Integrate Yetus AVRO-1887 Integrate Yetus Nov 5, 2018
@Fokko Fokko changed the title AVRO-1887 Integrate Yetus AVRO-1887 Integrate Yetus and fix tests Nov 5, 2018
@Fokko
Copy link
Contributor Author

Fokko commented Nov 5, 2018

@kojiromike @cutting PTAL.

I know there are a crazy amount of changes. Instead of having fixed paths, I updated all the tests to use TemporaryDirectories using junit. Also, I've rewritten a lot of the file operations using try-with-resource. A lot of the streams weren't closed at all.

@Fokko
Copy link
Contributor Author

Fokko commented Nov 5, 2018

@kojiromike My suggestion would be to merge this first, and then you can freely rework the shell scripts. You're much better at it than me :-)

Signed-off-by: sacharya <suraj.spa@gmail.com>
@Fokko Fokko force-pushed the fd-fix-tests branch 2 times, most recently from 2d234b0 to 4807128 Compare November 7, 2018 10:53
Add a precommit hook using Apache Yetus that will invoke the test
suite of the different languages

- Disable Ruby integration tests tests
- Fix Flaky Java Datetime test
  When the milsecons would have trailing zero, it would get trimmed
- Align the order of imports
@Fokko Fokko merged commit 25032e1 into apache:master Nov 7, 2018
@Fokko Fokko deleted the fd-fix-tests branch November 7, 2018 12:52
This was referenced Nov 7, 2018
@kojiromike
Copy link
Contributor

@Fokko Congratulations/Thanks for getting this merged. I'll take a look at those shell scripts. Am I correct that now we can rely on automated checks to validate a lot of our PRs? Is there a way to trigger the tests on all the open PRs?

@Fokko
Copy link
Contributor Author

Fokko commented Nov 7, 2018

@kojiromike with pleasure. They need to rebase onto master, then Travis will run automatically. I'm going through a couple of the PR's that are still open.

@aw-was-here
Copy link

FWIW, very specific support for Travis (and other CI systems) is something that will hopefully be coming to Yetus soon. I've got it in a fork already: effectivemachines/buretoolbox@f055aa3

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.

5 participants