Skip to content

docs(readme): use after() over ready() in example#157

Open
hungnb94 wants to merge 2 commits intofastify:mainfrom
hungnb94:dev
Open

docs(readme): use after() over ready() in example#157
hungnb94 wants to merge 2 commits intofastify:mainfrom
hungnb94:dev

Conversation

@hungnb94
Copy link

This PR addresses an issue where using the ready method causes the application to get stuck when running unit tests. To resolve this, the after method is used instead, ensuring that the necessary logic is executed without blocking the test execution.
Changes:

Replaced `ready` method with `after` to prevent test execution from getting stuck.

Reason for Change:

The `ready` method can introduce unexpected behavior in unit test environments, leading to tests hanging indefinitely.
The `after` method ensures the required logic is executed after initialization without interfering with test execution.

Testing:

Ran unit tests successfully without any hanging issues.
Verified the functionality remains unchanged in the main application.

Use after instead of ready to run test

Signed-off-by: hungnb94 <hungnb94@gmail.com>
@Fdawgs Fdawgs requested a review from kibertoad February 16, 2025 11:43
@Fdawgs Fdawgs changed the title Update README.md docs(readme): use after() over ready() in example Feb 16, 2025
@DmitryScaletta
Copy link

There is also wrong variable name in the example.
Should be:

-fastify.register(fastifySchedulePlugin);
+fastify.register(fastifySchedule);

@hungnb94
Copy link
Author

There is also wrong variable name in the example. Should be:

-fastify.register(fastifySchedulePlugin);
+fastify.register(fastifySchedule);

You're right. I'm going to update PR now.

(cherry picked from commit b83f7c4)
@Fdawgs
Copy link
Member

Fdawgs commented Feb 27, 2025

@hungnb94, the unit tests in this repo use ready() and they appear okay. Could you clarify on the issue, maybe with an example? Thanks!

@hungnb94
Copy link
Author

@hungnb94, the unit tests in this repo use ready() and they appear okay. Could you clarify on the issue, maybe with an example? Thanks!

You can check branch dev here: https://github.com/hungnb94/fastify-demo/tree/dev
I think the problem related to AutoLoad feature

Comment on lines +36 to +37
// Therefore, you need to call `after` method.
fastify.after().then(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

I think we discussed to remove the after() method to sponsor more the promise-way of doing it.

fastify/avvio#118 (comment)

So we should remove it from docs example instead

@Eomm Eomm added the documentation Improvements or additions to documentation label Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants