Skip to content

fix(lib/babe): integration tests bad merge#2814

Closed
qdm12 wants to merge 5 commits intodevelopmentfrom
qdm12/lib/babe/fix-compil-tests
Closed

fix(lib/babe): integration tests bad merge#2814
qdm12 wants to merge 5 commits intodevelopmentfrom
qdm12/lib/babe/fix-compil-tests

Conversation

@qdm12
Copy link
Copy Markdown
Contributor

@qdm12 qdm12 commented Sep 9, 2022

Changes

Just the occasional error due to merging without being forced to rebase/update your branch. Changes are:

  • Fix babe integration tests compilation
  • Fix babe integration tests mock configuration
  • Fix runtime error received Timestamp must be updated only once in the block

Tests

Issues

Primary Reviewer

@kishansagathiya

kishansagathiya and others added 4 commits September 9, 2022 20:15
- Split out (CGO related) helper functions from `imports.go` to `lib/runtime/wasmer/helpers.go`
- Move pointer size helper functions to `lib/runtime/wasmer/helpers.go`
- Change `toWasmMemorySized` to NOT take a size argument (unneeded)
- Clarify all comments for helper functions
- Update all error wrappings
- Review variable names
  - Use `ptr` instead of `out` for 32 bit pointers
  - Use `pointerSize` instead of `span` for 64 bit pointers size
  - Name return values
  - Other minor renamings such as `res` to `result`, `enc` to `encodedResult`
- Optimizations:
  - `storageAppend`: use slice capacity allocation, `copy` and remove unneeded `append`s
  - `toKillStorageResultEnum`: use `copy` instead of `append`
  - `toWasmMemoryOptional`: remove unneeded variable copy
@qdm12 qdm12 marked this pull request as ready for review September 9, 2022 14:30
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 9, 2022

Codecov Report

Merging #2814 (45ab288) into development (7fa9196) will increase coverage by 0.25%.
The diff coverage is n/a.

Additional details and impacted files
@@               Coverage Diff               @@
##           development    #2814      +/-   ##
===============================================
+ Coverage        62.97%   63.23%   +0.25%     
===============================================
  Files              213      213              
  Lines            27012    27012              
===============================================
+ Hits             17012    17081      +69     
+ Misses            8455     8384      -71     
- Partials          1545     1547       +2     

@qdm12 qdm12 changed the title fix(lib/babe): integration tests compilation fix(lib/babe): integration tests bad merge Sep 12, 2022
@qdm12
Copy link
Copy Markdown
Contributor Author

qdm12 commented Sep 12, 2022

@kishansagathiya running

go test -run ^TestService_HandleSlotWithSameSlot$ github.com/ChainSafe/gossamer/lib/babe -v -tags integration

Causes target=runtime message=panicked at 'Timestamp must be updated only once in the block', /home/elizabeth/substrate/frame/timestamp/src/lib.rs:197:13 ext_logging_log_version_1 pkg=runtime module=go-wasmer and cannot build inherents: running runtime function: Failed to call the BlockBuilder_apply_extrinsic exported function. do you have an idea how to fix it 🤔?

@qdm12
Copy link
Copy Markdown
Contributor Author

qdm12 commented Sep 12, 2022

Converting this back to draft since commit from #2726 got dropped. Let me know Kishan if/when I can close this & delete the branch.

@qdm12 qdm12 marked this pull request as draft September 12, 2022 19:43
@qdm12
Copy link
Copy Markdown
Contributor Author

qdm12 commented Nov 2, 2022

No longer needed

@qdm12 qdm12 closed this Nov 2, 2022
@qdm12 qdm12 deleted the qdm12/lib/babe/fix-compil-tests branch November 2, 2022 17:52
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.

2 participants