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

Upload test results to the database #4897

Merged
merged 1 commit into from Nov 11, 2022
Merged

Upload test results to the database #4897

merged 1 commit into from Nov 11, 2022

Conversation

akuzm
Copy link
Member

@akuzm akuzm commented Oct 28, 2022

This will help determine the most flaky tests.

@github-actions
Copy link

@fabriziomello, @pmwkaa: please review this pull request.

Powered by pull-review

@akuzm
Copy link
Member Author

akuzm commented Oct 28, 2022

@fabriziomello, @pmwkaa: please review this pull request.

Powered by pull-review

Oh wow, what is this stuff 😮 Please disregard it.

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #4897 (7c9aa33) into main (043bd55) will decrease coverage by 1.37%.
The diff coverage is 90.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4897      +/-   ##
==========================================
- Coverage   90.90%   89.52%   -1.38%     
==========================================
  Files         224      226       +2     
  Lines       42806    50999    +8193     
==========================================
+ Hits        38912    45659    +6747     
- Misses       3894     5340    +1446     
Impacted Files Coverage Δ
src/chunk.h 100.00% <ø> (ø)
src/chunk_index.h 100.00% <ø> (ø)
src/compat/compat.h 50.84% <ø> (-42.14%) ⬇️
src/dimension_vector.c 87.50% <0.00%> (+1.13%) ⬆️
src/guc.c 94.00% <0.00%> (-6.00%) ⬇️
src/nodes/chunk_append/exec.c 93.99% <ø> (-0.51%) ⬇️
src/nodes/chunk_append/planner.c 93.08% <ø> (-0.82%) ⬇️
src/nodes/hypertable_modify.c 35.78% <0.00%> (-34.51%) ⬇️
src/planner/planner.h 100.00% <ø> (ø)
src/ts_catalog/catalog.h 100.00% <ø> (ø)
... and 310 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f132148...7c9aa33. Read the comment docs.

@akuzm akuzm mentioned this pull request Nov 3, 2022
@akuzm akuzm marked this pull request as ready for review November 3, 2022 11:07
Comment on lines 72 to 75
- name: Get date for build caching
id: get-date
run: |
echo "date=$(date +"%m")" >> $GITHUB_OUTPUT
Copy link
Member

Choose a reason for hiding this comment

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

hmm i think weekly or even daily would be better

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to daily.

test_duration float
);

create unique index on test(job_date, test_name);
Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's convenient to have unique names for our tests. We had some duplicates (e.g. an isolation and a regression test with the same name), so I renamed them.

print buf[(NR + i - to_print) % max_context_lines]
}

printf("c %04d: %s\n", NR, $0);
Copy link
Member Author

Choose a reason for hiding this comment

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

The c %04d: part is for debugging. I'd like to leave it in for a while, until I'm absolutely sure that this diff snipping code works. It doesn't harm readability too much, and actually helps notice errors, because it has line numbers.

@konskov konskov self-requested a review November 9, 2022 08:54
@svenklemm
Copy link
Member

Hmm not a fan of this being in the matrix builder, can we split this into separate file to keep the matrix builder cleaner

@akuzm
Copy link
Member Author

akuzm commented Nov 9, 2022

Hmm not a fan of this being in the matrix builder, can we split this into separate file to keep the matrix builder cleaner

You mean, the flaky check? I don't know, logically it has to be somewhere around the matrix builder because it adds a special configuration and computes the installcheck options. I can split out the calculation of the changed tests into a separate procedure. Or even into a separate file, although to my taste that's an overkill.

Sorry for this PR being confusing, I already do three things here -- fixing flaky check, fixing the LLVM options like you asked me before, and finally adding the upload itself. Maybe it's better to split it up.

@akuzm
Copy link
Member Author

akuzm commented Nov 9, 2022

I already do three things here

Four things, also changing how the Postgres builds are cached.

@akuzm
Copy link
Member Author

akuzm commented Nov 9, 2022


with open(os.environ['GITHUB_OUTPUT'], 'a') as output:
print('regression_diff=true', file=output)
find . -name regression.diffs -exec cat {} + > regression.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the python code here? for convenience or is there another reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to make it as similar as possible to the other checks like Linux. This way it's easier to maintain, I can just diff those files and copy the fragments of the code, no need to write same thing anew in another language.

@akuzm akuzm force-pushed the aku/ci-db branch 8 times, most recently from b892f47 to 04a08cf Compare November 11, 2022 15:29
This will help us find the flaky tests or the rare failures.
@akuzm akuzm enabled auto-merge (rebase) November 11, 2022 15:47
@akuzm akuzm merged commit 252cefb into main Nov 11, 2022
@akuzm akuzm deleted the aku/ci-db branch November 11, 2022 16:02
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.

None yet

3 participants