Skip to content
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

Figuring out why tests fail on main #214

Merged
merged 5 commits into from
Oct 25, 2023
Merged

Conversation

Ngalstyan4
Copy link
Contributor

@Ngalstyan4 Ngalstyan4 commented Oct 25, 2023

Currently CI runs fail once a PR is merged, even though all tests pass while the change is an open PR.

The culprit is the recently added version update script. It seems somehow repo tags exist while the CI/CD is running on a pr but the repo tags are missing once CI/CD is running on the merged commit on main.

This made me think that the checkout action by default does not pull tag info and this PR attempts to fix it.

@var77, Do you think this the issue? Do you think my changes would fix it?
Is there a way to test this before we merge?

@github-actions
Copy link

github-actions bot commented Oct 25, 2023

Benchmarks

metric old new pct change
recall (after create) 0.740 0.740 -
recall (after insert) 0.754 0.754 -
select bulk tps 478.074 482.816 +0.99%
select bulk latency (ms) 15.882 15.576 -1.93%
select bulk latency (stddev ms) 4.022 2.480 -38.34%
create latency (ms) 1202.729 1201.081 -0.14%
insert bulk tps 10.998 11.509 +4.65%
insert bulk latency (ms) 90.922 86.882 -4.44%
insert bulk latency (stddev ms) 2.878 2.812 -2.29%
disk usage (bytes) 6348800.000 6348800.000 -

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Merging #214 (0f3bc04) into main (dda7f06) will increase coverage by 0.10%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #214      +/-   ##
==========================================
+ Coverage   83.06%   83.17%   +0.10%     
==========================================
  Files          18       18              
  Lines        1234     1236       +2     
  Branches      264      264              
==========================================
+ Hits         1025     1028       +3     
  Misses         83       83              
+ Partials      126      125       -1     
Files Coverage Δ
src/hnsw/options.c 87.93% <100.00%> (+0.21%) ⬆️
src/hnsw/scan.c 81.55% <100.00%> (+0.18%) ⬆️

... and 1 file with indirect coverage changes

@Ngalstyan4 Ngalstyan4 requested a review from var77 October 25, 2023 04:24
@var77
Copy link
Collaborator

var77 commented Oct 25, 2023

That is strange, I had added a line to fetch from remote in python script before, but seems it's not helping here. I can test this in my fork later today and let you know

@var77
Copy link
Collaborator

var77 commented Oct 25, 2023

@Ngalstyan4 in my fork here it is passing the tests in main

@Ngalstyan4 Ngalstyan4 merged commit cd8186c into main Oct 25, 2023
@Ngalstyan4 Ngalstyan4 deleted the narek/fetch-in-updates branch October 25, 2023 08:53
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.

2 participants