You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by "pratapaditya04 (via GitHub)" <gi...@apache.org> on 2024/09/24 09:17:38 UTC
[PR] [GOBBLIN-2160] added some unit tests for gobblin temporal module [gobblin]
pratapaditya04 opened a new pull request, #4059:
URL: https://github.com/apache/gobblin/pull/4059
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
### JIRA
- [ ] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-2160
### Description
- [ ] Here are some details about my PR, including screenshots (if applicable):
Added Unit tests for JobStateUtils and AbstractNestingExecWorkflowImpl classes in Gobblin-temporal module
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2160] Setup Temporal docker Cluster for CI Tests and added tests for AbstractNestingExecWorkflowImpl [gobblin]
Posted by "ImstyleX783 (via GitHub)" <gi...@apache.org>.
ImstyleX783 commented on PR #4059:
URL: https://github.com/apache/gobblin/pull/4059#issuecomment-6432545990
https://www.ouribank.com/O site oficial do Ouribank é:
- https://www.ouribank.com/
O canal digital/portal do banco que aparece como referência oficial é:
- https://www.ouribank.com/ (site institucional)
- Canal no YouTube da marca: https://www.youtube.com/c/BancoOurinvestcanal
Se quiser, também posso te ajudar a localizar:
- área de login do cliente,
- página de conta internacional,
- comparação entre Ouribank e bancos digitais como Wise/ Revolut / Nomad.Se você está procurando os canais digitais das maiores e mais importantes instituições financeiras globais, a lista abaixo reúne os sites oficiais dos maiores bancos do mundo por ativos e relevância internacional.
Os links direcionam direto para os respectivos portais institucionais e de serviços:
🇨🇳 Os Gigantes da China (Maiores do mundo em ativos)
• ICBC (Industrial and Commercial Bank of China): Site Oficial do ICBC
• China Construction Bank (CCB): Site Oficial do CCB
• Agricultural Bank of China (ABC): Site Oficial do ABC
• Bank of China: Site Oficial do Bank of China
🇺🇸 Os Líderes de Mercado dos Estados Unidos
• JPMorgan Chase (O banco mais valioso do mundo): Site Oficial do JPMorgan Chase
• Bank of America: Site Oficial do Bank of America
• Citigroup: Site Oficial do Citigroup
• Wells Fargo: Site Oficial do Wells Fargo
🇪🇺 Principais Bancos da Europa e Reino Unido
• BNP Paribas (França - Maior banco europeu): Site Oficial do BNP Paribas
• HSBC Holdings (Reino Unido): Site Oficial do HSBC
• Crédit Agricole (França): Site Oficial do Crédit Agricole
• Banco Santander (Espanha): Site Oficial do Santander Global
🌐 Contas Internacionais e Digitais Globais
Para quem busca abrir conta fora do país de forma 100% online:
• Wise (Conta multimoedas global): Site Oficial da Wise
• Revolut (Gigante digital britânico): Site Oficial da Revolut
• Nomad (Conta digital focada em dólar/EUA): Site Oficial da Nomad
🗺️ Diretórios Completos por País
Se você precisa pesquisar bancos de um país específico, existem duas grandes bases de dados mundiais de referência:
• Para indicadores macroeconômicos e dados de desenvolvimento por país, você pode consultar o portal oficial do Banco Mundial através do World Bank Open Data.
• Para visualizar uma lista estruturada e atualizada de bancos comerciais licenciados em mais de 100 países, acesse o guia do Global Brands Magazine Banks Directory.
Para te ajudar a encontrar o canal correto, você está procurando um banco de algum país específico ou quer informações sobre como abrir uma conta internacional?
Ouribank
Ouribank | O banco que abre o mundo
O Banco Ourinvest agora é Ouribank, evoluimos nossa marca para transformar fronteiras em novas possibilidades para você e para sua empresa.
Forbes Brasil
Os melhores bancos do mundo: Nubank é o primeiro entre os dez ...
8 de jun. de 2020 — 10. Bradesco Ano de fundação: 1943 País: Brasil CEO: Octavio de Lazari Funcionários: 819 Sede: São · 9. Santander Ano de fundação: 2013 País: Espanha CEO: José ...
Wise
Banco digital internacional: qual o melhor e como abrir conta online - Wise
21 de out. de 2022 — * Bancos digitais int.: menor burocracia, taxas reduzidas, sem agências físicas. * Comparativo contas (Grátis | Multimoeda | Disp. BR): - Wise: Sim | Sim | Sim ...
Euro Dicas
Melhor banco digital: confira o nosso top 6
* Wise: fundada 2011; conta multimoeda (>40 moedas); transferências (>160 países); sem agência. * Nomad: conta EUA em dólares; investimentos EUA; sem agência. *
World Bank Open Data
World Bank Open Data
World Bank Open Data * Investing in people through education, health, social protection, and gender equality. 87% Primary completion rate, female (% of relevant...
Wikipedia
Lista dos maiores bancos - Wikipédia
Conteúdo traduzido — Por ativos totais. editar. Sede do Banco Industrial e Comercial da China , o maior banco do mundo em ativos totais. Esta lista é baseada no relatório de abril d...
Suno - Investimentos Inteligentes
Bancos mais valiosos do mundo em 2025: descubra quais são - Suno
29 de ago. de 2024 — Ranking dos maiores bancos do mundo por valor de mercado. Posição, Banco, País, Valor de Mercado (US$ bilhões). 1º, JPMorgan Chase, EUA, 679 · 10 – HSBC Holding...
Investopedia
Os 10 maiores bancos do mundo - Investopedia
Conteúdo traduzido — Os cinco maiores bancos em valor de mercado são JPMorgan Chase, Bank of America, Agricultural Bank of China, Industrial and Commercial Bank of China (ICBC) e Ch...
Spherical Insights
Top 10 maior on-line Banking no mundo: crescimento, tamanho ...
1. JPMorgan Chase (Estados Unidos da América). JPMorgan Chase. A partir de 2023, a JPMorgan Chase tem cerca de 59 milhões de usuários ativos digitalmente, torna...
www.relbanks.com
Os 100 maiores bancos do mundo - Relbanks.com
Conteúdo traduzido — Os 100 maiores bancos do mundo. A seguir, apresentamos uma lista dos maiores bancos do mundo, classificados por ativos totais no final de 2025, compilada pela S...
Global Brands Magazine
List of Banks in the World by Country (2026 Directory)
Use this page as a starting point, then open any country for the full, verified list of banks. This is the Global Brands Magazine list of banks by country — a g...
Wikipedia
Lista dos maiores bancos dos Estados Unidos - Wikipédia
Table_title: Lista dos maiores bancos dos Estados Unidos Table_content: | Posição | Nome do banco | Cidade sede | | --- | --- | --- | | 1 | JP Morgan Chase | milhares de livros 📚
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2160] Setup Temporal docker Cluster for CI Tests and added tests for AbstractNestingExecWorkflowImpl [gobblin]
Posted by "Blazer-007 (via GitHub)" <gi...@apache.org>.
Blazer-007 commented on code in PR #4059:
URL: https://github.com/apache/gobblin/pull/4059#discussion_r1837451181
##########
gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/workflow/impl/AbstractNestingExecWorkflowImplTest.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gobblin.temporal.ddm.workflow.impl;
+
+import java.util.Optional;
+import java.util.UUID;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import io.temporal.api.common.v1.WorkflowExecution;
+import io.temporal.api.history.v1.History;
+import io.temporal.api.history.v1.HistoryEvent;
+import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryReverseRequest;
+import io.temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryReverseResponse;
+import io.temporal.client.WorkflowClient;
+import io.temporal.client.WorkflowOptions;
+import io.temporal.serviceclient.WorkflowServiceStubs;
+import io.temporal.worker.Worker;
+import io.temporal.worker.WorkerFactory;
+import io.temporal.api.enums.v1.EventType;
+
+
+import org.apache.gobblin.temporal.loadgen.activity.IllustrationItemActivity;
+import org.apache.gobblin.temporal.loadgen.work.IllustrationItem;
+import org.apache.gobblin.temporal.loadgen.work.SimpleGeneratedWorkload;
+import org.apache.gobblin.temporal.loadgen.workflow.impl.NestingExecOfIllustrationItemActivityWorkflowImpl;
+import org.apache.gobblin.temporal.util.nesting.work.WorkflowAddr;
+import org.apache.gobblin.temporal.util.nesting.work.Workload;
+import org.apache.gobblin.temporal.util.nesting.workflow.NestingExecWorkflow;
+
+
+public class AbstractNestingExecWorkflowImplTest {
+
+ private static final Logger logger = LoggerFactory.getLogger(AbstractNestingExecWorkflowImplTest.class);
+
Review Comment:
I don't think we need to log / or print something in tests, can we please remove them ?
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [GOBBLIN-2160] added some unit tests for gobblin temporal module [gobblin]
Posted by "khandelwal-prateek (via GitHub)" <gi...@apache.org>.
khandelwal-prateek commented on code in PR #4059:
URL: https://github.com/apache/gobblin/pull/4059#discussion_r1773794344
##########
gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/workflow/impl/AbstractNestingExecWorkflowImplTest.java:
##########
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.temporal.ddm.workflow.impl;
Review Comment:
update package to `org.apache.gobblin.temporal.util.nesting.workflow`
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/util/nesting/workflow/AbstractNestingExecWorkflowImpl.java:
##########
@@ -130,11 +134,9 @@ protected Duration calcPauseDurationBeforeCreatingSubTree(int numDirectLeavesChi
* List<Integer> naiveUniformity = Collections.nCopies(numSubTreesPerSubTree, numSubTreeChildren);
* @return each sub-tree's desired size, in ascending sub-tree order
*/
- protected static List<Integer> consolidateSubTreeGrandChildren(
- final int numSubTreesPerSubTree,
- final int numChildrenTotal,
- final int numSubTreeChildren
- ) {
+ @VisibleForTesting
+ public static List<Integer> consolidateSubTreeGrandChildren(final int numSubTreesPerSubTree,
Review Comment:
I understand that you added `@VisibleForTesting` and changed modifier to public for testing this method. However, this change is not required if we follow the same package structure for the test class and we can access protected methods without exposing them publicly. We can revert the modifier change and place the test class in the same package instead.
##########
gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/workflow/impl/AbstractNestingExecWorkflowImplTest.java:
##########
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.temporal.ddm.workflow.impl;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Optional;
+
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import io.temporal.workflow.Async;
+import io.temporal.workflow.Promise;
+import io.temporal.workflow.Workflow;
+
+import org.apache.gobblin.temporal.util.nesting.work.WorkflowAddr;
+import org.apache.gobblin.temporal.util.nesting.work.Workload;
+import org.apache.gobblin.temporal.util.nesting.workflow.AbstractNestingExecWorkflowImpl;
+
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(Workflow.class)
+public class AbstractNestingExecWorkflowImplTest {
+
+ @Mock
+ private Workload<String> mockWorkload;
+
+ @Mock
+ private WorkflowAddr mockWorkflowAddr;
+
+ @Mock
+ private Workload.WorkSpan<String> mockWorkSpan;
+
+ @Mock
+ private Promise<Object> mockPromise;
+
+ private AbstractNestingExecWorkflowImpl<String, Object> workflow;
+
+ @BeforeClass
+ public void setup() {
+ // PowerMockito is required to mock static methods in the Workflow class
+ Mockito.mockStatic(Workflow.class);
+ Mockito.mockStatic(Async.class);
+ Mockito.mockStatic(Promise.class);
+ this.mockWorkload = Mockito.mock(Workload.class);
+ this.mockWorkflowAddr = Mockito.mock(WorkflowAddr.class);
+ this.mockWorkSpan = Mockito.mock(Workload.WorkSpan.class);
+ this.mockPromise = Mockito.mock(Promise.class);
+
+ workflow = new AbstractNestingExecWorkflowImpl<String, Object>() {
+ @Override
+ protected Promise<Object> launchAsyncActivity(String task) {
+ return mockPromise;
+ }
+ };
+ }
+
+ @Test
+ public void testPerformWorkload_NoWorkSpan() {
+ // Arrange
+ Mockito.when(mockWorkload.getSpan(Mockito.anyInt(), Mockito.anyInt())).thenReturn(Optional.empty());
+
+ // Act
+ int result = workflow.performWorkload(mockWorkflowAddr, mockWorkload, 0, 10, 5, Optional.empty());
+
+ // Assert
+ Assert.assertEquals(0, result);
+ Mockito.verify(mockWorkload, Mockito.times(2)).getSpan(0, 5);
+ }
+
+ @Test
+ public void testCalcPauseDurationBeforeCreatingSubTree_NoPause() {
+ // Act
+ Duration result = workflow.calcPauseDurationBeforeCreatingSubTree(50);
+
+ // Assert
+ Assert.assertEquals(Duration.ZERO, result);
+ }
+
+ @Test
+ public void testCalcPauseDurationBeforeCreatingSubTree_PauseRequired() {
+ // Act
+ Duration result = workflow.calcPauseDurationBeforeCreatingSubTree(150);
+
+ // Assert
+ Assert.assertEquals(
+ Duration.ofSeconds(AbstractNestingExecWorkflowImpl.NUM_SECONDS_TO_PAUSE_BEFORE_CREATING_SUB_TREE_DEFAULT),
+ result);
+ }
+
+ @Test
+ public void testConsolidateSubTreeGrandChildren() {
+ // Act
+ List<Integer> result = AbstractNestingExecWorkflowImpl.consolidateSubTreeGrandChildren(3, 10, 2);
+
+ // Assert
+ Assert.assertEquals(3, result.size());
+ Assert.assertEquals(Integer.valueOf(0), result.get(0));
+ Assert.assertEquals(Integer.valueOf(0), result.get(1));
+ Assert.assertEquals(Integer.valueOf(6), result.get(2));
+ }
+
+ @Test(expectedExceptions = AssertionError.class)
+ public void testPerformWorkload_LaunchesChildWorkflows() {
Review Comment:
I see you've covered some scenarios, like handling an empty WorkSpan and testing the pause duration. However, the current tests don't fully capture the functionality(creating child workflows, subtrees handling, result calculation and edge cases) of the performWorkload method. It would be useful to add overall coverage for a method when we are adding tests for it, this ensures that the tests fulfill the original intent of thoroughly validating the behavior of missing tests. This would also help avoid the need for additional work on this method again later, unless we are making changes to it.
--
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: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org