Skip to content

Processor unit tests#15

Merged
Guozhang Wang (guozhangwang) merged 5 commits into
confluentinc:streamingfrom
guozhangwang:ProcessorUnitTests
Aug 28, 2015
Merged

Processor unit tests#15
Guozhang Wang (guozhangwang) merged 5 commits into
confluentinc:streamingfrom
guozhangwang:ProcessorUnitTests

Conversation

@guozhangwang

Copy link
Copy Markdown

ymatsuda

"./gradlew streaming:jar" succeeds again, finally.

  1. Fix the StreamTaskTest.
  2. Fix ProcessorJob example code.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Didn't this break a KStream test? KStream tests use MockProcessorContext.
Why did you move joinable() from ProcessorContext to KStreamJoin?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I feel this function is specific to KStream not in Processor, as its checking logic is sort-of specific to KStream's definitions. Which tests need MockProcessorContext.joinable() calls?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

KStreamJoin should not care how the joinability is computed. So, having joinable() in ProcessorContext is actually good.

Anything that makes tests harder is bad since it is an indication of a bad code organization.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Join is not a KStream specific concept.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OK, will change it then.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

clean up imports

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

There is no unused imports, I will group o.a.k.s.kstream and o.a.k.s.processor.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, I meant the clean up after you move joinable() back to ProcessContext.

@guozhangwang

Copy link
Copy Markdown
Author

ymatsuda

Now "./gradlew stream:test" works, but 10 out of 29 tests fails.. Could we work in parallel to fix them after checking this in?

@ymatsuda

Copy link
Copy Markdown

OK. Let's merge your PR.

Guozhang Wang (guozhangwang) added a commit that referenced this pull request Aug 28, 2015
@guozhangwang Guozhang Wang (guozhangwang) merged commit 8ccab76 into confluentinc:streaming Aug 28, 2015
@guozhangwang Guozhang Wang (guozhangwang) deleted the ProcessorUnitTests branch September 14, 2015 19:33
José Armando García Sancio (jsancio) pushed a commit that referenced this pull request Sep 13, 2019
It is useful to know what the majority of failed authentications' causes are. This way, we can better understand what is happening with the system and gauge whether they might be intentional or not.

Reviewers: Ismael, Rajini
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