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

Directly embed lib.TestPreInitState in js/common.InitEnvironment #2999

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

na--
Copy link
Member

@na-- na-- commented Mar 31, 2023

Instead of manually including most of its constituent parts, we can just embed the whole *lib.TestPreInitState in the struct. This will make any future changes much easier.

I also tried to move FileSystems and CWD in lib.TestPreInitState, so we can get rid of them in both cmd.loadedTest and common.InitEnvironment (at that point, we could basically replace it with lib.TestPreInitState directly):

k6/cmd/test_load.go

Lines 31 to 39 in c09919b

// loadedTest contains all of data, details and dependencies of a loaded
// k6 test, but without any config consolidation.
type loadedTest struct {
sourceRootPath string // contains the raw string the user supplied
pwd string
source *loader.SourceData
fs afero.Fs
fileSystems map[string]afero.Fs
preInitState *lib.TestPreInitState

Unfortunately, FileSystems is complicated, since when we are running .js files, its elements contain the "cache-on-read" overlay filesystems on top of the real FS (and on HTTPs), while for .tar archives we substitute them with the filesystems in the archive:

k6/js/bundle.go

Lines 134 to 137 in c09919b

return newBundle(piState, &loader.SourceData{
Data: arc.Data,
URL: arc.FilenameURL,
}, arc.Filesystems, arc.Options, false)

So, I decided to leave this for a future PR. With some further refactoring of the js test loading, it should be possible to handle in a nicer way than it currently is. Potentially before #2975 🤔

@github-actions github-actions bot requested review from imiric and oleiade March 31, 2023 12:09
@na-- na-- requested review from mstoykov and codebien March 31, 2023 12:09
@na-- na-- changed the title Duirectly include lib.TestPreInitState in js/common.InitEnvironment Directly include lib.TestPreInitState in js/common.InitEnvironment Mar 31, 2023
Base automatically changed from linter-mega-fixes to master March 31, 2023 12:31
@na-- na-- force-pushed the pre-init-state-in-vu-init branch from 74faa3c to ed32098 Compare March 31, 2023 12:32
@na-- na-- changed the title Directly include lib.TestPreInitState in js/common.InitEnvironment Directly embed lib.TestPreInitState in js/common.InitEnvironment Mar 31, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #2999 (c8ff1e0) into master (988f2f6) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head c8ff1e0 differs from pull request most recent head ed32098. Consider uploading reports for the commit ed32098 to get more accurate results

@@            Coverage Diff             @@
##           master    #2999      +/-   ##
==========================================
+ Coverage   76.99%   77.02%   +0.02%     
==========================================
  Files         228      228              
  Lines       17050    17050              
==========================================
+ Hits        13128    13132       +4     
+ Misses       3079     3076       -3     
+ Partials      843      842       -1     
Flag Coverage Δ
ubuntu 77.02% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
js/common/initenv.go 66.66% <ø> (ø)
js/bundle.go 84.57% <100.00%> (-0.16%) ⬇️
js/modulestest/runtime.go 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@na-- na-- removed request for oleiade and imiric March 31, 2023 13:07
@na-- na-- merged commit e402652 into master Apr 4, 2023
@na-- na-- deleted the pre-init-state-in-vu-init branch April 4, 2023 07:44
ka3de added a commit to grafana/xk6-browser that referenced this pull request Apr 4, 2023
An incompatibility was detected with k6 after grafana/k6#2999.
This commit fixes this incompatibility, which mainly affected structures
defined in k6 browser tests.
ka3de added a commit to grafana/xk6-browser that referenced this pull request Apr 4, 2023
An incompatibility was detected with k6 after grafana/k6#2999.
This commit updates k6 dependency to latest version and fixes the
present incompatibility, which mainly affects structures defined in k6
browser tests.
ka3de added a commit to grafana/xk6-browser that referenced this pull request Apr 4, 2023
An incompatibility was detected with k6 after grafana/k6#2999.
This commit updates k6 dependency to latest version and fixes the
present incompatibility, which mainly affects structures defined in k6
browser tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants