Skip to content

Conversation

@darkgl0w
Copy link
Member

@darkgl0w darkgl0w commented Dec 4, 2021

Hello.
This PR aims to :

Checklist

@darkgl0w
Copy link
Member Author

darkgl0w commented Dec 4, 2021

Hmmmm GitHub CI Windows runners failure 🤔
Should not be related to the PR IMO: cf. this commit CI run

@mcollina
Copy link
Member

mcollina commented Dec 4, 2021

CI is failing :(

@darkgl0w
Copy link
Member Author

darkgl0w commented Dec 4, 2021

CI is failing :(

Can you manually trigger a CI re-run please ?
I suspect this to be a problem on GitHub CI Windows runner side, If it fails again I will try to see what is triggering this issue ^^

@darkgl0w
Copy link
Member Author

darkgl0w commented Dec 4, 2021

It seems that there is a flacky test when running in the Windows environment.
cf. https://github.com/fastify/fastify-caching/runs/4376801490?check_suite_focus=true and https://github.com/fastify/fastify-caching/runs/4376801490?check_suite_focus=true 🤔

I need to go, but I will try to look at this later today or tomorrow.

@darkgl0w
Copy link
Member Author

darkgl0w commented Dec 4, 2021

OK it seems good now (I hope 😸), I ended up rewriting the tests (I swapped http.get calls with fastify.inject and I have gone async/await syntax), let me know if I need to change or revert anything. 😛

I have triggered a few CI run cycles and it seems to be OK so 🤞.
For reference https://github.com/darkgl0w/fastify-caching/actions

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Eomm Eomm merged commit 1d7b828 into fastify:master Dec 5, 2021
@Eomm
Copy link
Member

Eomm commented Dec 5, 2021

@darkgl0w the merge into master has some issue 😞

test count(2) != plan(3)

https://github.com/fastify/fastify-caching/actions/runs/1541922678

Could you take a look?

@darkgl0w
Copy link
Member Author

darkgl0w commented Dec 5, 2021

@Eomm > It seems that the root cause of the test flackyness is still there in Windows environments.
I will take a look tomorrow morning.

Edit: PR sent thanks to GitHub.dev vscode !_!

@darkgl0w darkgl0w mentioned this pull request Dec 5, 2021
4 tasks
@darkgl0w darkgl0w deleted the remove-preparsing-hooks branch December 5, 2021 19:40
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.

4 participants