search

microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
23.02k stars 6.35k forks source link

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

Open wrobelda opened 4 years ago

wrobelda commented 4 years ago

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. 😢
BillyONeal commented 4 years ago

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 commented 3 years ago

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

wrobelda commented 3 years ago

@BillyONeal this bit me again. https://github.com/microsoft/vcpkg/pull/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 https://github.com/microsoft/vcpkg/pull/18116.

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

JackBoosY commented 3 years ago

@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 commented 3 years ago

@JackBoosY @BillyONeal this issue should remain open for as long as this has not been resolved. It continues (https://github.com/microsoft/vcpkg/pull/19945#issuecomment-916868283) 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).