Skip to content

Directory tests #204

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.

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

Closed
wants to merge 25 commits into from
Closed

Directory tests #204

wants to merge 25 commits into from

Conversation

Kishan-Dhakan
Copy link
Contributor

Re-open PR for dir-tests to investigate fails.

@stewartie4
Copy link
Contributor

these weren't failing initially - strange that they're failing now unless something has suddenly broken?

@Kishan-Dhakan
Copy link
Contributor Author

@cnlangzi please don't merge this upon passing yet, I had moved some cases to Broken scenarios so I need to move them back, Thanks

@cnlangzi
Copy link
Contributor

cnlangzi commented Mar 16, 2022

@Kishan-Dhakan other related issues are raised on 0chain/blobber#590, 0chain/blobber#592 and 0chain/blobber#588

@Kishan-Dhakan Kishan-Dhakan linked an issue Apr 7, 2022 that may be closed by this pull request
6 tasks
@stewartie4
Copy link
Contributor

this looks ok to review again @Kishan-Dhakan @mohsenno1 right?

@Kishan-Dhakan
Copy link
Contributor Author

@stewartie4 yep, OOMKilled issue is solved but almost every case is a FIXME. If it's not an issue, this should be ready to merge

@stewartie4
Copy link
Contributor

Maybe we should move these files to broken scenarios so we can better keep track on them

@stewartie4
Copy link
Contributor

where are we with this @mohsenno1?

@Kishan-Dhakan
Copy link
Contributor Author

@stewartie4 I fixed the tests but directory operations are not working at the moment. I think this will be done after mainnet, see: https://0chain.slack.com/archives/C035RK9PTU0/p1652452527605049?thread_ts=1652443141.811229&cid=C035RK9PTU0.

cc @cnlangzi please let us know whether coverage for directory operations is needed at the moment

allocationID := setupAllocation(t, configPath)
defer createAllocationTestTeardown(t, allocationID)

dirname := "/rootdir"
Copy link
Contributor

Choose a reason for hiding this comment

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

/rootdir is not root directory. / is.

@lpoli
Copy link
Contributor

lpoli commented May 23, 2022

@stewartie4 @Kishan-Dhakan
Lets not merge anything by tagging it as FIXME or as Broken Test. This will delay bug fixing.

Its better to raise issue and link respective system test PR and assign to somebody in the blobber/gosdk team.

@cnlangzi
Copy link
Contributor

https://0chain.slack.com/archives/C035RK9PTU0/p1652452527605049?thread_ts=1652443141.811229&cid=C035RK9PTU0

Yes. I would suggest to implement directory features/commands after mainnet. Because I think it is not good to implement it as a normal synchronous restful api

@stewartie4
Copy link
Contributor

@cnlangzi agreed, multiple files in an allocation would be very taxing on server side
@lpoli agreed

@Kishan-Dhakan
Copy link
Contributor Author

@lpoli @cnlangzi @stewartie4 great, we're on the same page.

@Kishan-Dhakan
Copy link
Contributor Author

Closing this, needs to be reopened post-mainnet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase directory test coverage
5 participants