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

updated integration tests to use gasschedule v4 #3798

Merged
merged 12 commits into from
Feb 17, 2022

Conversation

raduchis
Copy link
Contributor

just integration test changes

@codecov
Copy link

codecov bot commented Feb 15, 2022

Codecov Report

Merging #3798 (51b1392) into development (bffe14d) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 51b1392 differs from pull request most recent head 5831e8a. Consider uploading reports for the commit 5831e8a to get more accurate results

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3798      +/-   ##
===============================================
- Coverage        74.62%   74.62%   -0.01%     
===============================================
  Files              606      606              
  Lines            79672    79672              
===============================================
- Hits             59457    59455       -2     
- Misses           15640    15641       +1     
- Partials          4575     4576       +1     
Impacted Files Coverage Δ
p2p/libp2p/netMessenger.go 78.86% <0.00%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 25949ec...5831e8a. Read the comment docs.

@@ -308,7 +309,7 @@ func TestMultipleTimesERC20BigIntInBatches(t *testing.T) {
t.Skip("this is not a short test")
}

gasSchedule, _ := common.LoadGasScheduleConfig("../../../../cmd/node/config/gasSchedules/gasScheduleV2.toml")
gasSchedule, _ := common.LoadGasScheduleConfig(arwen.GasSchedulePath)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -1,3 +1,4 @@
//go:build !race
Copy link
Contributor

Choose a reason for hiding this comment

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

Arwen should be ok now. Maybe remove these pre-processor lines along with the following TODO?
Valid on all files

Copy link
Contributor Author

@raduchis raduchis Feb 15, 2022

Choose a reason for hiding this comment

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

removed from all files in integrationstests/vm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems not to be ready yet... undid the remove

@@ -66,7 +66,7 @@ var oneShardCoordinator = mock.NewMultiShardsCoordinatorMock(2)
var pkConverter, _ = pubkeyConverter.NewHexPubkeyConverter(32)

// GasSchedulePath --
var GasSchedulePath = "../../../../cmd/node/config/gasSchedules/gasScheduleV2.toml"
var GasSchedulePath = "../../../../cmd/node/config/gasSchedules/gasScheduleV4.toml"
Copy link
Contributor

Choose a reason for hiding this comment

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

this util package.... I think we can move this var (that can be a const, actually) in the integrationTests/testInitializer.go and we can slowly dissolve this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

sasurobert
sasurobert previously approved these changes Feb 15, 2022
@@ -118,12 +120,12 @@ func TestDelegation_Claims(t *testing.T) {
context.GasLimit = 30000000
err = context.ExecuteSC(&context.Alice, "claimRewards")
require.Nil(t, err)
require.Equal(t, 22313926, int(context.LastConsumedFee))
require.Equal(t, 8148760, int(context.LastConsumedFee))
Copy link
Collaborator

Choose a reason for hiding this comment

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

🚀

@@ -20,9 +22,9 @@ func TestDNS_Register(t *testing.T) {

var empty struct{}
arwen.DNSAddresses[string(expectedDNSAddress)] = empty
arwen.GasSchedulePath = "../../../cmd/node/config/gasSchedules/gasScheduleV2.toml"
gasScheduleConfigPath := strings.ReplaceAll(integrationTests.GasSchedulePath, "../../../..", "../../..")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Quite tricky.

We could have used filepath.Abs() in the definition of integrationTests.GasSchedulePath and never worry about relative paths when using it in other packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

talk about it, and can remain like this for now

@raduchis raduchis merged commit 437a5d7 into development Feb 17, 2022
@raduchis raduchis deleted the update-tests-gasschedulev4 branch February 17, 2022 14:56
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.

None yet

5 participants