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.

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

Remove Updated Difficulty Tests from BasicTests Folder #976

Merged
merged 2 commits into from
Nov 1, 2021

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Oct 31, 2021

This PR removes the BasicTests folder which has been relocated to the ethereum/legacytests repo.

Update: Only remove difficulty tests that are already updated in #977.

@winsvega
Copy link
Collaborator

winsvega commented Oct 31, 2021

there are some files that might still be used by someone.
those are really legacy tests, cause it was here before the frontier even

Copy link
Collaborator

@winsvega winsvega left a comment

Choose a reason for hiding this comment

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

lets move to legacy only the difficulty tests
legacytests root / BasicTests / *olddifficulty tests

@@ -1,20 +0,0 @@
[
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@@ -1,16 +0,0 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@@ -1,5 +0,0 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@@ -1,62 +0,0 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@@ -1,22 +0,0 @@
[
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@@ -1,24 +0,0 @@
[
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea how many teams are still using this files. perhaps it is better to leave it here as it is small

@holiman
Copy link
Contributor

holiman commented Nov 1, 2021

As for go-ethereum, we use the BasicTests in this file: https://github.com/ethereum/go-ethereum/blob/master/tests/difficulty_test.go#L84 -- it is the difficultyTestDir.

We skip these:

	dt.skipLoad("hexencodetest.*")
	dt.skipLoad("crypto.*")
	dt.skipLoad("blockgenesistest\\.json")
	dt.skipLoad("genesishashestest\\.json")
	dt.skipLoad("keyaddrtest\\.json")
	dt.skipLoad("txtest\\.json")

And these:

	// files are 2 years old, contains strange values
	dt.skipLoad("difficultyCustomHomestead\\.json")
	dt.skipLoad("difficultyMorden\\.json")
	dt.skipLoad("difficultyOlimpic\\.json")

@holiman
Copy link
Contributor

holiman commented Nov 1, 2021

For all changes such as these, please make sure to write them up on the release notes, eventually, so we can make sure to update and use the new paths.

@winsvega
Copy link
Collaborator

winsvega commented Nov 1, 2021

yes. the existing difficulty tests we plan to move to LegacyTests subfolder (repo)
and have a copy of them in a new difficulty test format #970
with new tests to cover arrowGlacier as well.

JavaScript team reported that they still use some of the tests from this folder

@marioevz
Copy link
Member Author

marioevz commented Nov 1, 2021

I think that partially removing some of the files might end up being more confusing to the users instead of simply instructing:
"./BasicTests is now at ./LegacyTests/BasicTests, be sure to add '--recurse-submodules' when cloning this repo to obtain these files if you require them", or something similar.
Let me know what you think @holiman @winsvega

@winsvega
Copy link
Collaborator

winsvega commented Nov 1, 2021

for thise little files it does not make much sense to fetch the whole legacy tests repo.
removing old difficulty tests to legacy repo is what we want. it will make everyone notice that there are new difficulty tests

@marioevz
Copy link
Member Author

marioevz commented Nov 1, 2021

Got it, sounds good. Will remove only what's being updated in #977, everything else remains in ./BasicTests .
For ethereum/legacytests#1, I added everything under ./BasicTests -> ./LegacyTests/BasicTests, but will update to only add files removed in this PR.

@marioevz marioevz changed the title Remove BasicTests Folder Remove Updated Difficulty Tests from BasicTests Folder Nov 1, 2021
@winsvega winsvega merged commit 035891a into develop Nov 1, 2021
@winsvega winsvega deleted the removeBasicTests branch November 1, 2021 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants