You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "kidylee (via GitHub)" <gi...@apache.org> on 2023/03/30 15:32:46 UTC

[GitHub] [incubator-opendal] kidylee opened a new pull request, #1809: feat(bindings/java): add cucumber test case for memory

kidylee opened a new pull request, #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809

   Memory and File system are working ok.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1809: feat(bindings/java): add cucumber test case for memory

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809#discussion_r1153927122


##########
bindings/java/src/test/java/org/apache/opendal/StepsTest.java:
##########
@@ -0,0 +1,58 @@
+package org.apache.opendal;
+
+import io.cucumber.java.en.Given;
+import io.cucumber.java.en.Then;
+import io.cucumber.java.en.When;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class StepsTest {
+
+    Operator operator;
+
+    @Given("A new OpenDAL Blocking Operator")
+    public void a_new_open_dal_blocking_operator() {
+        Map<String, String> params = new HashMap<>();
+        params.put("root", "/tmp");
+        this.operator = new Operator("Memory", params);
+    }
+
+    @When("Blocking write path \"test\" with content \"Hello, World!\"")
+    public void blocking_write_path_test_with_content_hello_world() {
+        this.operator.write("test", "Hello, World!");
+    }
+
+
+    @Then("The blocking file \"test\" should exist")
+    public void the_blocking_file_test_should_exist() {
+
+    }
+
+
+    @Then("The blocking file \"test\" entry mode must be file")
+    public void the_blocking_file_test_entry_mode_must_be_file() {
+        // Write code here that turns the phrase above into concrete actions
+//        throw new io.cucumber.java.PendingException();
+    }
+
+    @Then("The blocking file \"test\" content length must be 13")
+    public void the_blocking_file_test_content_length_must_be_13() {
+        // Write code here that turns the phrase above into concrete actions

Review Comment:
   Please remove not useful comment



##########
bindings/java/src/test/java/org/apache/opendal/operator/OperatorTest.java:
##########


Review Comment:
   Please delete this file as we will not be adding any additional tests for bindings (for now).



##########
bindings/java/src/test/java/org/apache/opendal/StepsTest.java:
##########
@@ -0,0 +1,58 @@
+package org.apache.opendal;
+
+import io.cucumber.java.en.Given;
+import io.cucumber.java.en.Then;
+import io.cucumber.java.en.When;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class StepsTest {
+
+    Operator operator;
+
+    @Given("A new OpenDAL Blocking Operator")
+    public void a_new_open_dal_blocking_operator() {
+        Map<String, String> params = new HashMap<>();
+        params.put("root", "/tmp");
+        this.operator = new Operator("Memory", params);
+    }
+
+    @When("Blocking write path \"test\" with content \"Hello, World!\"")
+    public void blocking_write_path_test_with_content_hello_world() {
+        this.operator.write("test", "Hello, World!");
+    }
+
+
+    @Then("The blocking file \"test\" should exist")
+    public void the_blocking_file_test_should_exist() {
+
+    }
+
+
+    @Then("The blocking file \"test\" entry mode must be file")
+    public void the_blocking_file_test_entry_mode_must_be_file() {
+        // Write code here that turns the phrase above into concrete actions
+//        throw new io.cucumber.java.PendingException();
+    }
+
+    @Then("The blocking file \"test\" content length must be 13")
+    public void the_blocking_file_test_content_length_must_be_13() {
+        // Write code here that turns the phrase above into concrete actions
+        String content = this.operator.read("test");

Review Comment:
   Does `cucumber` support capturing the value we input? It would be better to refer to a variable instead of manually writing `"test"` here.
   
   For example:
   
   https://github.com/apache/incubator-opendal/blob/85c3785d5ff2543278325788916da43fe2dc91e9/bindings/python/tests/steps/binding.py#L38-L50



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1809: feat(bindings/java): add cucumber test case for memory

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809#discussion_r1153931986


##########
bindings/java/src/lib.rs:
##########
@@ -38,10 +38,21 @@ pub extern "system" fn Java_org_apache_opendal_Operator_getOperator(
         .get_string(&input)
         .expect("Couldn't get java string!")
         .into();
+
     let schema = Scheme::from_str(&input).unwrap();

Review Comment:
   > Hello, please refrain from raising reviews related to formatting. These errors should be caught by our CI system. If the CI system fails to catch them, it indicates a bug in our infrastructure.
   
   Oh, sorry, my fault. It's a typo, not just format issue.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo merged pull request #1809: feat(bindings/java): add cucumber test

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] suyanhanx commented on a diff in pull request #1809: feat(bindings/java): add cucumber test case for memory

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809#discussion_r1153927644


##########
bindings/java/src/lib.rs:
##########
@@ -38,10 +38,21 @@ pub extern "system" fn Java_org_apache_opendal_Operator_getOperator(
         .get_string(&input)
         .expect("Couldn't get java string!")
         .into();
+
     let schema = Scheme::from_str(&input).unwrap();

Review Comment:
   ```suggestion
        let scheme = Scheme::from_str(&input).unwrap();
   ```
   



##########
bindings/java/src/test/java/org/apache/opendal/StepsTest.java:
##########
@@ -0,0 +1,58 @@
+package org.apache.opendal;
+
+import io.cucumber.java.en.Given;
+import io.cucumber.java.en.Then;
+import io.cucumber.java.en.When;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class StepsTest {
+
+    Operator operator;
+
+    @Given("A new OpenDAL Blocking Operator")
+    public void a_new_open_dal_blocking_operator() {
+        Map<String, String> params = new HashMap<>();
+        params.put("root", "/tmp");
+        this.operator = new Operator("Memory", params);
+    }
+
+    @When("Blocking write path \"test\" with content \"Hello, World!\"")
+    public void blocking_write_path_test_with_content_hello_world() {
+        this.operator.write("test", "Hello, World!");
+    }
+
+
+    @Then("The blocking file \"test\" should exist")
+    public void the_blocking_file_test_should_exist() {
+
+    }
+
+
+    @Then("The blocking file \"test\" entry mode must be file")
+    public void the_blocking_file_test_entry_mode_must_be_file() {
+        // Write code here that turns the phrase above into concrete actions
+//        throw new io.cucumber.java.PendingException();
+    }
+
+    @Then("The blocking file \"test\" content length must be 13")
+    public void the_blocking_file_test_content_length_must_be_13() {
+        // Write code here that turns the phrase above into concrete actions
+        String content = this.operator.read("test");

Review Comment:
   We should use the var which is injected.
   
   https://cucumber.io/docs/cucumber/step-definitions/?lang=java



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on pull request #1809: feat(bindings/java): add cucumber test case for memory

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809#issuecomment-1491159527

   Mostly LGTM, please fix the license check :love_letter: 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1809: feat(bindings/java): add cucumber test case for memory

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #1809:
URL: https://github.com/apache/incubator-opendal/pull/1809#discussion_r1153931785


##########
bindings/java/src/lib.rs:
##########
@@ -38,10 +38,21 @@ pub extern "system" fn Java_org_apache_opendal_Operator_getOperator(
         .get_string(&input)
         .expect("Couldn't get java string!")
         .into();
+
     let schema = Scheme::from_str(&input).unwrap();

Review Comment:
   Hello, please refrain from raising reviews related to formatting. These errors should be caught by our CI system. If the CI system fails to catch them, it indicates a bug in our infrastructure.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org