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.

Sign up for GitHub

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

Jump to bottom

Invalidate old testing-farm request IDs and split up chroot comments #380

Merged
merged 29 commits into from
Apr 10, 2024
Merged

Invalidate old testing-farm request IDs and split up chroot comments #380

merged 29 commits into from
Apr 10, 2024

Conversation

kwk
Copy link
Collaborator

@kwk kwk commented Apr 3, 2024 •
edited
Loading

This PR does many things.

  • Invalidate old testing-farm request IDs if the Copr build IDs have changed.
    • Still allow old testing-farm HTML comments without any Copr build IDs and update them on the fly.
  • Add testing-farm tests
  • Exclude testing-farm-logs dir from pre-commit check
  • Flip_test_label improvement and tests
  • Fix error with authentication
  • Fix Incomplete URL substring sanitization
  • Render errors for each chroot in their own updatetable and hidable comment with a link in
    the chroot column of the build matrix.
    • Added ability to hide and unhide (minimize/unminimize) comments through GraphQL
    • Hide/Unhide all unused/used chroot error comments if they belong to a chroot we don't care about
  • Fix rendering of test output
  • Reorganize testing-farm bits
  • Fix type hints in python code
  • Shorten testing-farm log

@kwk
Copy link
Collaborator Author

kwk commented Apr 3, 2024

Consider the chroot fedora-38-x86_64 only having successful builds. What happens is, that we kick of testing farm. But then you think that you need to change something and you kick of a new build in the fedora-38-x86_64 chroot again and it succeeds. In that case the testing-farm output will stay which is wrong by design. I not only need to log the chroot and request ID of testing farm but I need to log the old build IDs with it in order to properly invalidate any testing-farm output.

kwk
kwk commented Apr 4, 2024
View reviewed changes
snapshot_manager/snapshot_manager/testing_farm_util.py
chroot: str
"""The chroot for which this testing-farm request was issues"""

copr_build_ids: list[int]
Copy link
Collaborator Author

@kwk kwk Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Consider the chroot fedora-38-x86_64 only having successful builds. What happens is, that we kick of testing farm. But then you think that you need to change something and you kick of a new build in the fedora-38-x86_64 chroot again and it succeeds. In that case the testing-farm output will stay which is wrong by design. I not only need to log the chroot and request ID of testing farm but I need to log the old build IDs with it in order to properly invalidate any testing-farm output.

The Copr build IDs are now associated with a TestingFarmRequest object.

@kwk kwk changed the title Invalidate old testing-farm request IDs Invalidate old testing-farm request IDs and split up chroot comments Apr 8, 2024
@kwk kwk self-assigned this Apr 8, 2024
@kwk kwk added enhancement New feature or request pip python dependencies Pull requests that update a dependency file labels Apr 8, 2024
kwk added a commit that referenced this pull request Apr 8, 2024
@kwk
Fix Incomplete URL substring sanitization
2512c56 
The string `"artifacts.osci.redhat.com`  may be at an arbitrary position in the sanitized URL.

Resolve: #380 (review)
@kwk kwk force-pushed the invalidate-testing-farm-request-ids branch from 2512c56 to 68233d2 Compare April 8, 2024 09:17
@kwk kwk force-pushed the invalidate-testing-farm-request-ids branch 3 times, most recently from 26b8e4d to a02574d Compare April 8, 2024 09:40
kwk added 15 commits April 9, 2024 13:21
@kwk
Invalidate old testing-farm request IDs
e8a7bd7
@kwk
Fix type hints
ea12159
@kwk
Reorganize testing-farm bits
ec56cfd
@kwk
More improvements
cd3cc2a
@kwk
Allow flip_test_label to remove all testing labels
965da78
@kwk
Add labels all at once
8c6e991
@kwk
All working
59af8e1 
* splitting errors into their own distinct comments
* links from the build matrix to the error comments
* and more fixes
@kwk
Fix rendering of test output
9614914
@kwk
Forgot to add the HTML marker to the chroot error comment before
6c69b4e
@kwk
Fix rendering of tests
7ceb545
@kwk
fix typo
d839fc0
@kwk
Added ability to hide comment through GraphQL
f4bc24d 
We use this in order to hide chroot comments for which no errors show
up, hence they are outdated.
@kwk
Fix Incomplete URL substring sanitization
8f963b7 
The string `"artifacts.osci.redhat.com`  may be at an arbitrary position in the sanitized URL.

See https://github.com/fedora-llvm-team/llvm-snapshots/security/code-scanning/2
@kwk
Try to fix error with authentication
7338c00
@kwk
flip_test_label improvement and tests
95f86b5
@kwk kwk force-pushed the invalidate-testing-farm-request-ids branch from 4237837 to dc75756 Compare April 9, 2024 11:21
kwk added 2 commits April 9, 2024 13:28
@kwk
Update test_flip_test_label
b7dddf5
@kwk
Attempt to fix IndexError in test
50ede5d 
```
 snapshot_manager/tests/github_util_test.py:69:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <github.PaginatedList.PaginatedList object at 0x7f60c5d27e00>, index = 0

    def __getitem__(self, index: Union[int, slice]) -> Any:
        assert isinstance(index, (int, slice))
        if isinstance(index, int):
            self.__fetchToIndex(index)
>           return self.__elements[index]
E           IndexError: list index out of range

/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/github/PaginatedList.py:77: IndexError
```
@kwk kwk marked this pull request as ready for review April 9, 2024 11:50
@kwk kwk force-pushed the invalidate-testing-farm-request-ids branch from 39c8a9c to 1d0cc61 Compare April 10, 2024 23:07
@kwk kwk merged commit 95ebdf0 into main Apr 10, 2024
8 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kwk kwk

Labels
dependencies Pull requests that update a dependency file enhancement New feature or request pip python testing-farm tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@kwk