Skip to content

Fix false positive tests in CI for Windows - #598

Merged
f0rmiga merged 2 commits into
bazel-contrib:mainfrom
UebelAndre:windows
Feb 11, 2022
Merged

Fix false positive tests in CI for Windows#598
f0rmiga merged 2 commits into
bazel-contrib:mainfrom
UebelAndre:windows

Conversation

@UebelAndre

@UebelAndreUebelAndre commented Jan 7, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Currently the integration test runner does not report errors for windows.

Issue Number: N/A

What is the new behavior?

The test runner has been fixed and the examples/tests updated to be compatible with windows.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This PR includes the following which are both required to get passing windows builds.

@UebelAndre
UebelAndre marked this pull request as ready for review January 7, 2022 04:56
UebelAndre
UebelAndre commented Jan 7, 2022
View reviewed changes

@UebelAndreUebelAndre left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@jheaff1 would you mind taking a look to see if I missed anything? And maybe help me address some TODOs? 😅 🙏

Comment threadexamples/pip_install/BUILD Outdated
Comment threadexamples/pip_install/pip_install_test.py Outdated
Comment threadexamples/pip_parse/pip_parse_test.py Outdated
Comment threadversion.bzl Outdated
@UebelAndreUebelAndre changed the title Reorganied tests to fix testing on windowsReorganized tests to fix testing on windowsJan 7, 2022
@UebelAndreUebelAndre mentioned this pull request Jan 10, 2022
Closed
12 tasks
@jheaff1

jheaff1 commented Jan 15, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

I’m suprised the diff_tests works in CI, as diff_test is broken when using —-enable_runfiles until skylib v1.1.1

bazelbuild/bazel-skylib@43a545a

@UebelAndre

UebelAndre commented Jan 15, 2022

Copy link
Copy Markdown
ContributorAuthor

@jheaff1 Thanks so much for your help!

@jheaff1

jheaff1 commented Jan 16, 2022

Copy link
Copy Markdown
Contributor

@jheaff1 Thanks so much for your help!

My pleasure, although you did 99% of the work, so thank you! 😂

@UebelAndreUebelAndre mentioned this pull request Jan 19, 2022
Closed
13 tasks
UebelAndre referenced this pull request Jan 19, 2022
@UebelAndre
Added support for annotating rendered pip dependencies (#589)
7740b22
This was referenced Jan 29, 2022
Merged
Merged
@thundergolfer
thundergolfer removed the request for review from andyscottFebruary 1, 2022 11:10
@UebelAndreUebelAndre changed the title Reorganized tests to fix testing on windowsFix false positive tests in CI for WindowsFeb 10, 2022
@UebelAndre

UebelAndre commented Feb 10, 2022

Copy link
Copy Markdown
ContributorAuthor

As all other changes have been merged, this PR is just left with #597

@f0rmiga
f0rmiga merged commit d80a9aa into bazel-contrib:mainFeb 11, 2022
@UebelAndre
UebelAndre deleted the windows branch February 11, 2022 01:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandjonbrandjonAwaiting requested review from brandjon
@lberkilberkiAwaiting requested review from lberki
@thundergolferthundergolferAwaiting requested review from thundergolfer
2 more reviewers
@jheaff1jheaff1jheaff1 left review comments
@f0rmigaf0rmigaf0rmiga approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@UebelAndre@jheaff1@f0rmiga