Skip to content

matching estimates #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Oct 13, 2023
Merged

matching estimates #142

merged 19 commits into from
Oct 13, 2023

Conversation

vacekj
Copy link
Contributor

@vacekj vacekj commented Jul 9, 2023

frontend pr: gitcoinco/grants-stack#2022
ticket: #307

@vacekj vacekj force-pushed the match-estimates branch 4 times, most recently from 7cea331 to 42a7d90 Compare July 24, 2023 10:13
@vacekj vacekj force-pushed the match-estimates branch 2 times, most recently from 5e74d57 to 8188528 Compare September 29, 2023 09:02
@vacekj vacekj marked this pull request as ready for review September 29, 2023 10:51
@vacekj vacekj force-pushed the match-estimates branch 2 times, most recently from 3f7d404 to e9a79d8 Compare October 3, 2023 12:40
@vacekj vacekj changed the title wip: matching estimates matching estimates Oct 6, 2023
bard
bard previously approved these changes Oct 6, 2023
@vacekj
Copy link
Contributor Author

vacekj commented Oct 12, 2023

@boudra @bard there are two test failures that I'm not immediately sure how to resolve, I'll investigate tomorrow but it might be a quick fix for you.

@bard
Copy link
Contributor

bard commented Oct 12, 2023

@vacekj I've narrowed it down to tmpNameSync returning the same "temporary" (not so temporary) name twice in a row. Can be seen by console.logging inside PassportProvider.start. Switching to tmpName (async version) didn't help.

Cannot look into it further right now but to debug, you can console.log the dbPath inside PassportProvider.start — as long as it prints the same name more than once, the problem is there. To fix, tmpNameSync in src/passport/index.test.fixtures.ts needs to be replaced with something that actually generates different names.

@boudra
Copy link
Contributor

boudra commented Oct 12, 2023

I can look into it in half an hour, but if it helps i had this exact same problem in my branch when i updated Vitest, downgrading it fixed the problem, very strange.

@boudra
Copy link
Contributor

boudra commented Oct 12, 2023

I pushed a commit to rollback Vitest to 0.34.5 so that we can leave things as they were and we can make fix the passport issue separately since it might not be an obvious fix.

tmpNameSync seems to definitely be generating random paths, the issue I think is with test.extend which seems to be giving each test the same context instead of generating one every time, which maybe is a behaviour change or bug that was introduced in Vitest 0.34.6.

@gravityblast gravityblast merged commit c3c5a83 into main Oct 13, 2023
@gravityblast gravityblast deleted the match-estimates branch October 13, 2023 08:54
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.

Indexer: calculate matching estimates based on donation amount and QF algo
4 participants