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.

Sign up for GitHub

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

Jump to bottom

WIP: implement testing.TempDir, os.RemoveAll, and os.ReadDir. #2500

Closed
wants to merge 10 commits into from
Closed

WIP: implement testing.TempDir, os.RemoveAll, and os.ReadDir. #2500

wants to merge 10 commits into from

Conversation

dkegel-fastly
Copy link
Contributor

@dkegel-fastly dkegel-fastly commented Jan 9, 2022 •
edited
Loading

It slices! It dices! It's a floor wax and a shoe polish!

testing.TempDir() needs testing.Cleanup() needs os.RemoveAll needs os.ReadDir.
And its tests need sub-benchmarks.

Pull it all together into one pull request as a preview while we wait for the individual pull requests to land.

Only works on darwin and linux for now, because the current readdir patch does not handle windows.

@dkegel-fastly dkegel-fastly marked this pull request as draft January 9, 2022 02:54
@dkegel-fastly
Copy link
Contributor Author

dkegel-fastly commented Jan 9, 2022 •
edited
Loading

That's kind of fun -- with all those changes to testing, "go test -run TestBuild/Host" fails on mac.
Easy to check outside the test harness; just do

   cd testdata; tinygo run testing.go

with old and new trees, and compare output. I'll look into it when I get a chance.

@dkegel-fastly
Copy link
Contributor Author

dkegel-fastly commented Jan 9, 2022 •
edited
Loading

Evidently "testing: gather duplicate code into tRunner" introduced this regression. Checking...

@dkegel-fastly dkegel-fastly mentioned this pull request Jan 9, 2022
Open
@dkegel-fastly
Copy link
Contributor Author

OK, sorted. This passes now on CI on darwin, which is the only place I've implemented os.ReadDir so far.

@dkegel-fastly
Copy link
Contributor Author

The first two parts of this that are ready for review are:

I'll rebase this once those land.

@dkegel-fastly dkegel-fastly changed the title WIP: implement testing.TempDir, testing.Cleanup, os.RemoveAll, os.ReadDir, and sub-benchmarks. WIP: implement testing.TempDir, os.RemoveAll, and os.ReadDir. Jan 11, 2022
@dkegel-fastly
Copy link
Contributor Author

Rebased. Also include breadcrumbs pull request, since I use that, and this is just a preview of the pull requests I use, stacked together.

@dkegel-fastly
Copy link
Contributor Author

dkegel-fastly commented Jan 16, 2022 •
edited
Loading

I just tried a few tests that depend on these functions... didn't find any that now pass, but here are the immediate blockers I found (ignoring any tests immediately blocked by reflect).

cmd/addr2line - os.ExpandEnv
cmd/gofmt - runtime.NumCPU, f.Chmod 
cmd/nm - os.ExpandEnv
cmd/objdump - os.ExpandEnv
cmd/pack - syscall.RawSyscall
cmd/pprof - os.ExpandEnv
debug/gosym - syscall.RawSyscall
io/ioutil - IsNotExist bug
path/filepath - os.Symlink

@dkegel-fastly dkegel-fastly marked this pull request as ready for review January 23, 2022 01:36
@dkegel-fastly
Copy link
Contributor Author

This is in pretty good shape now.
#2571 should land first,
then I'll rebase #2499,
then I'll create a PR for os.RemoveAll,
then finally one for testing.TempDir.

@dkegel-fastly dkegel-fastly force-pushed the dkegel-WIP-justice-league branch 2 times, most recently from 248e192 to af35672 Compare January 23, 2022 02:58
@dkegel-fastly
Revert "Kludge: work around lack of syscall.seek on 386 and arm, tiny…
f77a7dd 
…go-org#1906"

This reverts commit 60b483b.
@dkegel-fastly
Kludge: provide stub for syscall.seek on 386 and arm, tinygo-org#1906
da08a89 
This replaces an earlier kludge which was at the wrong level
and caused "GOARCH=386 tinygo test os" to fail to compile on linux.

Stubbing just the one missing function, syscall.seek, lets
os tests compile on linux 386, and skipping tests of seek and Fstat
(which has a cryptic dependency on syscall.Seek via time)
lets os tests pass on linux 386.

The stub can be removed once tinygo implements go assembly and picks up the real definition.
@dkegel-fastly
os: implement readdir for darwin and linux
9bec207 
readdir is disabled on linux for 386 and arm until syscall.seek is implemented there.

windows is hard, so leaving that for later.

File src/os/dir_other_go115.go can be deleted when we drop support for go 1.15.
@dkegel-fastly
Implement os.RemoveAll()
231e058 
TODO: enable test on windows once Readdir is implemented there
@dkegel-fastly
os: provide stub for MkdirAll and RemoveAll in wasi
4b0529f
@dkegel-fastly
testing: implement testing.TempDir
026978f 
TODO: enable test on windows once os.Readdir and os.RemoveAll are implemented there
@dkegel-fastly
os: isWine: be compatible with older versions of wine, too
c630d0a 
Turns out wine 5.x doesn't export WINEUSERNAME.
@dkegel-fastly
os: add TestReadNonDir, fix problem it found on darwin
0260f01
@dkegel-fastly
os: add DirFS
eb600cc
@dkegel-fastly
os: disable TestClearenv for now, it is very flaky. Also reduce verbo…
d6cdcde 
…sity.
@dkegel-fastly dkegel-fastly marked this pull request as draft January 23, 2022 23:52
@dkegel-fastly
Copy link
Contributor Author

Closing in favor of individual pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@dkegel-fastly