You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by ToivoAdams <gi...@git.apache.org> on 2016/08/06 13:54:30 UTC
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
GitHub user ToivoAdams opened a pull request:
https://github.com/apache/nifi/pull/803
nifi-1214c Mock Framework should allow order-independent assumptions \u2026
\u2026on FlowFiles. This replaces previous nifi-121b
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ToivoAdams/nifi nifi-1214c
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/803.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #803
----
commit 36ed4690f40e1bf405d0644d8b3877cf50d5858c
Author: Toivo Adams <to...@gmail.com>
Date: 2016-08-06T13:46:01Z
nifi-1214c Mock Framework should allow order-independent assumptions on FlowFiles. This replaces previous nifi-121b
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
Posted by ToivoAdams <gi...@git.apache.org>.
Github user ToivoAdams commented on a diff in the pull request:
https://github.com/apache/nifi/pull/803#discussion_r79117053
--- Diff: nifi-mock/src/main/java/org/apache/nifi/util/MockFlowFile.java ---
@@ -290,4 +291,18 @@ public long getLineageStartIndex() {
public long getQueueDateIndex() {
return 0;
}
+
+ public boolean isAttributeEqual(final String attributeName, final String expectedValue) {
+ // unknown attribute name, so cannot be equal.
+ if (attributes.containsKey(attributeName) == false)
+ return false;
+
+ String value = attributes.get(attributeName);
+ return Objects.equals(expectedValue, value);
+ }
+
+ public boolean isContentEqual(String excpected) {
+ final String value = new String(this.data, Charset.forName("UTF-8"));
+ return Objects.equals(excpected, value);
--- End diff --
Hi @JPercivall,
Thanks for reviewing.
Maybe I misunderstood, but assertContentEquals methods don\u2019t return boolean and won\u2019t work with Predicate\u2019s.
For example, I can\u2019t use assertContentEquals here (instead of isContentEqual)
```java
runner.assertAllConditionsMet("success",
mff -> mff.isAttributeEqual("GROUP_ATTRIBUTE_KEY", "1") && mff.isContentEqual("1,hello\n1,good-bye")
);
```
isContentEqual parameter could be byte[] array.
But I think its convenient to have String parameter also, maybe 2 overloaded methods?
Thanks
Toivo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #803: nifi-1214c Mock Framework should allow order-independent as...
Posted by ToivoAdams <gi...@git.apache.org>.
Github user ToivoAdams commented on the issue:
https://github.com/apache/nifi/pull/803
New PR #1033
I did rebase as usual.
But git refused to push and recommended pull before push.
I did pull but it seems I should force push instead of pull.
Thanks
Toivo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #803: nifi-1214c Mock Framework should allow order-independent as...
Posted by ToivoAdams <gi...@git.apache.org>.
Github user ToivoAdams commented on the issue:
https://github.com/apache/nifi/pull/803
Git complained and I did what he recommend (git pull origin).
It seems this was mistake.
I will create another clean PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi/pull/803#discussion_r78998108
--- Diff: nifi-mock/src/main/java/org/apache/nifi/util/MockFlowFile.java ---
@@ -290,4 +291,18 @@ public long getLineageStartIndex() {
public long getQueueDateIndex() {
return 0;
}
+
+ public boolean isAttributeEqual(final String attributeName, final String expectedValue) {
+ // unknown attribute name, so cannot be equal.
+ if (attributes.containsKey(attributeName) == false)
+ return false;
+
+ String value = attributes.get(attributeName);
+ return Objects.equals(expectedValue, value);
+ }
+
+ public boolean isContentEqual(String excpected) {
+ final String value = new String(this.data, Charset.forName("UTF-8"));
+ return Objects.equals(excpected, value);
--- End diff --
Instead of comparing a String to binary data (and forcing the "UTF-8" character encoding), I think it would be better to have the parameter be a byte[] and do Arrays.equal() on them. Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
Posted by ToivoAdams <gi...@git.apache.org>.
Github user ToivoAdams closed the pull request at:
https://github.com/apache/nifi/pull/803
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #803: nifi-1214c Mock Framework should allow order-independent as...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/803
Hey @ToivoAdams, did you get a chance to make a new PR? I don't see one anywhere.
Also when updating your local branch, instead of using git "pull" use "rebase". Then when updating your PR branch use "-f" to force the update to your personal repo (never use "-f" or "rebase" when interacting with the Apache repo). Rebase makes for clean updates to PRs.
You can visually see the difference using this site: https://onlywei.github.io/explain-git-with-d3/#rebase
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi/pull/803#discussion_r79183117
--- Diff: nifi-mock/src/main/java/org/apache/nifi/util/MockFlowFile.java ---
@@ -290,4 +291,18 @@ public long getLineageStartIndex() {
public long getQueueDateIndex() {
return 0;
}
+
+ public boolean isAttributeEqual(final String attributeName, final String expectedValue) {
+ // unknown attribute name, so cannot be equal.
+ if (attributes.containsKey(attributeName) == false)
+ return false;
+
+ String value = attributes.get(attributeName);
+ return Objects.equals(expectedValue, value);
+ }
+
+ public boolean isContentEqual(String excpected) {
+ final String value = new String(this.data, Charset.forName("UTF-8"));
+ return Objects.equals(excpected, value);
--- End diff --
Ah sorry, you are totally right, isContentEqual is perfectly valid.
Yup, I agree that going with a couple overloaded methods (like assertContentEquals does) would be the right way to go.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #803: nifi-1214c Mock Framework should allow order-independent as...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/803
@ToivoAdams yup, it's complaining because when you "rebase" you are re-writing history. When you try to push it's out of sync with the branch you're pushing to and it tells you to "pull" in order to be coordinated. By doing a "force push", you're saying "forget whatever the current state of the branch is, what I'm pushing is correct".
This is very nice when you're doing it to your own branch/PR but it's pretty self-explanatory how bad it could be doing to the Apache repo.
I'm gonna take a look at the new PR now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #803: nifi-1214c Mock Framework should allow order-indepen...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on a diff in the pull request:
https://github.com/apache/nifi/pull/803#discussion_r79011075
--- Diff: nifi-mock/src/main/java/org/apache/nifi/util/MockFlowFile.java ---
@@ -290,4 +291,18 @@ public long getLineageStartIndex() {
public long getQueueDateIndex() {
return 0;
}
+
+ public boolean isAttributeEqual(final String attributeName, final String expectedValue) {
+ // unknown attribute name, so cannot be equal.
+ if (attributes.containsKey(attributeName) == false)
+ return false;
+
+ String value = attributes.get(attributeName);
+ return Objects.equals(expectedValue, value);
+ }
+
+ public boolean isContentEqual(String excpected) {
+ final String value = new String(this.data, Charset.forName("UTF-8"));
+ return Objects.equals(excpected, value);
--- End diff --
Actually I'm confused why this is added when "assertContentEquals"[1] and it's various overloading methods exist.
[1] https://github.com/apache/nifi/blob/master/nifi-mock/src/main/java/org/apache/nifi/util/MockFlowFile.java#L233-L233
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #803: nifi-1214c Mock Framework should allow order-independent as...
Posted by JPercivall <gi...@git.apache.org>.
Github user JPercivall commented on the issue:
https://github.com/apache/nifi/pull/803
Hey @ToivoAdams, sorry for the delay but I'm taking a look at this now.
Also, the "github" way to link to another pr is by referencing the pr number, like so: #388
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---