keith
Test: new test cases showing existing issues on windows
This PR adds unit tests for buildifier that fail to demonstrate the current issues with Windows.
Because running fix modifies the source tree, I set them up as bazel shell tests so that they run in temporary workspaces.
You can add this command to CI: bazel test //tests/buildifier/... --enable_runfiles
Although the tests must run with runfiles, the sub-workspaces that they validate run in both runfiles and non-runfiles mode.
It would be nice to re-enable the existing integration tests for windows on CI. However, rules_bazel_integration test is currently broken on windows - around half of the test cases are failing. Part of this might be its dependency on buildifier-prebuilt! I can have a look at that at some point, but for now I suggest we use these bazel shell tests to add coverage that works on all platforms, and I'll look at fixing rules_bazel_integration_test as a followup once my windows fixes here are released.
@keith I've pushed a fix for ubuntu. For windows, could you add --enable_runfiles to the test command? That is required.
Actually, I've fixed the new test so it can be used in norunfiles mode.
@keith, could you rerun CI please?
I've improved the test cases. I'd expect //tests:buildifier:buildifier_test to fail on both linux and windows. The linux failures are due to buildifier check/fix having some issue in norunfiles mode.
@keith could you rerun CI?
These are the messages reported from inside //tests:buildifier:buildifier_test On windows: ** 2 / 6 tests passed. ********************************************************* ** There were errors. **********************************************************
On linux: ** 4 / 6 tests passed. ********************************************************* ** There were errors. **********************************************************
Thank you. That is failing as expected. I've merged the latest state of this PR into #89 so we can evaluate those results. Please do whatever you see fit in terms of closing these separately or just as #89.
@cgrindel thank you, windows 👍 , issue with bazel 5 on linux, I've submitted a fix, could you rerun?
@peakschris I don't see an Approve button for CI. DId you push your changes?
@cgrindel it auto-ran. This CI is now good; passing linux, failing on windows. The windows failures are addressed by #89.
@peakschris I don't see an Approve button for CI. DId you push your changes?
I pushed it.
@cgrindel @keith I have a little time and can take a brief look at getting this and the windows fix PR in if you will have availability to review & kick off CI jobs?