Move api/filesys to kyaml/filesys

[campoy]

Updates #3873

This PR moves all of the code in api/filesys to kyaml/filesys and updates all known dependencies to use the new package.

Instead of simply removing api/filesys, I kept a deprecated package (not the Deprecated: comment) so users can migrate at their pace.

[k8s-ci-robot]

@campoy: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

Hi @campoy. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[campoy]

I just moved code from one package to another so I am afraid the tests breaking were breaking previously too for Windows.

[natasha41575]

/ok-to-test

[natasha41575]

I just moved code from one package to another so I am afraid the tests breaking were breaking previously too for Windows.

As a workaround, can we add // +build aix darwin dragonfly freebsd js,wasm linux netbsd openbsd solaris to the top of filesys_test.go and util_test.go (and any other test files that are failing the windows check)?

[campoy]

How about I just add something to skip windows?
So basically, // +build !windows?

[natasha41575]

How about I just add something to skip windows?
So basically, // +build !windows?

Sounds good to me! Thanks

[KnVerey]

I just moved code from one package to another so I am afraid the tests breaking were breaking previously too for Windows.

I agree this should not cause any new failures, and this is really strange. But master wasn't failing before, so how is this possible? Any ideas @natasha41575 ? Is there some other exclusion mechanism being used to skip the tests in their previous location?

[natasha41575]

I agree this should not cause any new failures, and this is really strange. But master wasn't failing before, so how is this possible? Any ideas @natasha41575 ? Is there some other exclusion mechanism being used to skip the tests in their previous location?

The windows check doesn't run all the tests for api, but does for kyaml. If you look at the check, it has jobs "Test kyaml" and "Test cmd/config", but no "Test api" which is odd. I'm not sure who set up these tests or if this is intentional.

[campoy]

I'm adding a test that will necessarily fail on api/filesys to see whether this was being tested at all.

[campoy]

This actually confirms that the "Go / Test Windows" precheck doesn't check 'api/filesys'

[natasha41575]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: campoy, natasha41575

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [natasha41575]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[KnVerey]

Why the change to not have a separate test package here and in a few other files?

[natasha41575]

I had the same question but (maybe incorrectly) assumed that it didn't matter. Was there a motivation for having them separate?

[KnVerey]

As far as I'm aware, the main reason for separate test packages is to force the tests to be written against the public API, i.e. test from the perspective of the consumers of the package. I can't speak to whether that was the reason for the separate package in this case. Since this PR is just moving code from one place to another, I think it's fair to ask for an explanation of for any actual changes that were included. :)