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

CI results should yield INCONCLUSIVE if ports that were changed could not be tested #13533

Open
wrobelda opened this issue Sep 15, 2020 · 5 comments
Assignees
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre

Comments

@wrobelda
Copy link
Contributor

wrobelda commented Sep 15, 2020

Describe the bug
Your CI currently returns Success results even if the ports that were changed as part of the PR were not actually being tested for whatever reason. I already got bit by it significantly and almost successfully pushed a non-functional PR that was previously LGTM-ed by two of the maintainers.

Please also note that some more devs who previously contributed to this project also were affected by this issue.

To Reproduce

  1. ./vcpkg install xyz
  2. See no errors
  3. Pull a request
  4. Elevate your sense of self-worth, for thine PR is error-free!
  5. Open a bottle and that pack of fresh nachos.
  6. Take a trip downstairs in the elevator. Casually flex your muscles in the mirrors. Camera? Let them watch.
  7. Get a face tattoo.
  8. Wake up in Bangkok

Expected behavior

  1. ./vcpkg install xyz
  2. See no errors.
  3. Pull a request.
  4. See errors.
  5. Enjoy your stale nachos while you fix them.
  6. Remember that you're not funny at all. 😢
@JackBoosY JackBoosY added the category:question This issue is a question label Sep 15, 2020
@BillyONeal
Copy link
Member

Generally speaking, the PR system doesn't know what ports you "edited" -- the way we currently figure out how they're edited is via the binary caching system. We find the hash of all the things, and rebuild anything which isn't in the cache.

That said, CASCADE results should really be in ci.baseline.txt; that would have caught this particular problem.

@JackBoosY
Copy link
Contributor

We hope your question was answered to your satisfaction; if it wasn't, you can reopen with more info.

@wrobelda
Copy link
Contributor Author

wrobelda commented May 27, 2021

@BillyONeal this bit me again. #16953 was showing CASCADE for windows static build, which I admittedly did not notice, nor did anyone else for that matter. It got merged and ended up causing all other PRs to fail, until fixed by #18116.

@JackBoosY I believe this issue should very much stay open, as it continues to be relevant and causes trouble.

@JackBoosY
Copy link
Contributor

JackBoosY commented May 31, 2021

@wrobelda This because getopt-win32:x64-windows-static is skipped:

         getopt-win32:x64-windows-static:       skip: 717614f156738dac672f484de025a1c493378a35
         getopt:x64-windows-static:             cascade: 2b0a67d57f7f4ee74eb6b987961aa14e2a340b79

In my opinion, ci.baseline.txt should be cleand to prevent this.

@wrobelda
Copy link
Contributor Author

wrobelda commented Sep 12, 2021

@JackBoosY @BillyONeal this issue should remain open for as long as this has not been resolved. It continues (#19945 (comment)) to mislead your contributors and not acknowledging the issue leads to frustration to say the least, not to mention the accidental approvals of PRs (as exemplified in my original comment).

@JackBoosY JackBoosY reopened this Sep 13, 2021
@JackBoosY JackBoosY added category:infrastructure Pertaining to the CI/Testing infrastrucutre and removed category:question This issue is a question labels May 27, 2022
@Cheney-W Cheney-W assigned Cheney-W and unassigned JackBoosY Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:infrastructure Pertaining to the CI/Testing infrastrucutre
Projects
None yet
Development

No branches or pull requests

4 participants