test: Implement GSP-86 Add linker integration tests #40

abyss-w · 2021-07-21T07:58:35Z

test: Implement GSP-86 Add linker integration tests

linker.go

Xuanwo · 2021-07-22T08:24:07Z

go.mod

@@ -3,7 +3,7 @@ module github.com/beyondstorage/go-integration-test/v4
 go 1.15

 require (
-	github.com/beyondstorage/go-storage/v4 v4.3.2
+	github.com/beyondstorage/go-storage/v4 v4.3.3-0.20210720033749-df6a96af6045


ping @JinnyYi, Let's solve beyondstorage/go-storage#682 so that we can use it here.

go-storage v4.4.0 have been released.

JinnyYi · 2021-07-22T14:01:03Z

I made some mistakes and tested symlink based on files in fs and objects in oss:

When target is exist and path is not exist:
- Both fs and oss can create symlink successfully and the returned symlink is readable.
When target is not exist and path is not exist:
- Both fs and oss can create symlink successfully.
- The target can be get from the symlink.
- After we write to target(the target file or object), we can get the content from the symlink.
When target is exist and path is also exist:
- For oss, PutSymlink will succeed no mater path identifies an object or a symlink, like overwrite.
- For fs, os.Symlink will get error no mater path identifies a file, a directory or a symlink.
For oss, the Symlink object type can be get from ListObject, but seems can't get form GetMetadata.
For fs, maybe we should check and delete the path if it is exist as describe in GSP-134: Write Behavior Consistency.

Xuanwo · 2021-07-23T05:11:01Z

Nice catch, maybe we need to add them in specs or update the rfc.

Xuanwo · 2021-07-26T02:40:55Z

Linker should also take #42 a look.

JinnyYi · 2021-07-26T15:18:00Z

linker.go

+	})
+}
+
+func TestLinkerWithVirtualLink(t *testing.T, store types.Storager) {


This function will be called by services that support overwrite, like oss. But actually symlink is native support for oss. So maybe the function name TestLinkerWithVirtualLink is not suitable?

How about using TestLinkerWithoutOverwrite and TestLinkerWithOverWrite as function names?

Xuanwo · 2021-07-27T03:26:49Z

This PR lives so long.

How about removing the parts we haven't agreed on yet and merge TestLinker into master first?

We can specify the corner case behavior later.

abyss-w · 2021-07-27T03:35:14Z

How about removing the parts we haven't agreed on yet and merge TestLinker into master first?

OK

Xuanwo · 2021-07-27T03:49:02Z

linker.go

+
+			Convey("Read should get path object data without error", func() {
+				var buf bytes.Buffer
+				n, err := store.Read(path, &buf)


LinkObject is not readable, so we can't read from a link directly. Instead, we need to read from the link object's LinkTarget.

Maybe we can remove tests about Read should get path object data without error?

Xuanwo · 2021-07-27T04:44:40Z

@abyss-w Hi, we will squash all commits while merging, so don't bother to do a force push here.

abyss-w · 2021-07-27T04:50:04Z

@abyss-w Hi, we will squash all commits while merging, so don't bother to do a force push here.

ok, got it.

Xuanwo · 2021-07-27T04:57:17Z

ping @JinnyYi to take another look.

Xuanwo · 2021-07-27T06:17:52Z

Thank you for your patient and detailed work!

test: Implement GSP-86 Add linker integration tests

4febc85

JinnyYi reviewed Jul 21, 2021

View reviewed changes