Skip to content

Conversation

XavierGeerinck
Copy link
Contributor

Signed-off-by: Xavier Geerinck xavier.geerinck@gmail.com

Description

Issue reference

Please reference the issue this PR will close: #177

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
@XavierGeerinck XavierGeerinck requested review from a team as code owners February 14, 2022 08:33
Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
@XavierGeerinck XavierGeerinck changed the title Add workflow adaptation to ensure build script finishes correctly Fix test sequence and dev container Feb 14, 2022
…'s a debug measurement

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
@XavierGeerinck
Copy link
Contributor Author

@shubham1172 can you review?

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
shubham1172
shubham1172 previously approved these changes Feb 14, 2022
Copy link
Member

@shubham1172 shubham1172 left a comment

Choose a reason for hiding this comment

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

Thanks @XavierGeerinck, LGTM overall! Just had a few nits/questions.

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
shubham1172
shubham1172 previously approved these changes Feb 14, 2022
@XavierGeerinck
Copy link
Contributor Author

XavierGeerinck commented Feb 14, 2022

Note: awaiting merging, it seems that test-e2e is timing out (13min run time and counting so cancelling it and checking it)

== APP ==   http/actors
== APP ==     actorProxy
== APP ==       ✓ should be able to create an actor object through the proxy (16 ms)
== APP ==     invoke
== APP ==       ✓ should register actors correctly (1 ms)
== APP ==       ✓ should be able to invoke an actor through a text message (2 ms)
== APP ==       ✓ should be able to invoke an actor through an object message (2 ms)
== APP ==       ✓ should be able to invoke an actor through multiple parameters (2 ms)
== APP ==       ✓ should be able to invoke an actor through the client which abstracts the actor proxy builder for people unaware of patterns (2 ms)
== APP ==     timers
== APP ==       ✓ should fire a timer correctly (expected execution time > 5s) (5020 ms)
== APP ==     reminders
== APP ==       ✓ should be able to unregister a reminder (4020 ms)
== APP ==       ✓ should fire a reminder but with a warning if it's not implemented correctly (2013 ms)
== APP == 
== APP == Test Suites: 1 passed, 1 total
== APP == Tests:       9 passed, 9 total
== APP == Snapshots:   0 total
== APP == Time:        17.935 s
== APP == Ran all test suites matching /test\/e2e\/actors.http.test.ts/i.
== APP == Jest did not exit one second after the test run has completed.
== APP == 
== APP == This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
Copy link
Member

@shubham1172 shubham1172 left a comment

Choose a reason for hiding this comment

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

Thanks @XavierGeerinck, I only had a few minor comments from the last change.

Signed-off-by: Xavier Geerinck <xavier.geerinck@gmail.com>
@shubham1172
Copy link
Member

Since we are no more sending lint output to /dev/null, should we create a work item to fix existing linting issues?

@XavierGeerinck
Copy link
Contributor Author

Since we are no more sending lint output to /dev/null, should we create a work item to fix existing linting issues?

We can yes, there are quite some warning items. I will create one

@XavierGeerinck XavierGeerinck merged commit b529036 into dapr:master Feb 15, 2022
@XavierGeerinck XavierGeerinck deleted the issue_177 branch February 15, 2022 11:09
@shubham1172 shubham1172 mentioned this pull request Feb 16, 2022
3 tasks
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.

Fix tests and build process for catching errors
2 participants