Test docs for plugins #18337
Test docs for plugins #18337
Conversation
nik9000
added
>docs
| @@ -290,20 +290,20 @@ class ClusterFormationTasks { | |||
| Copy copyConfig = project.tasks.create(name: name, type: Copy, dependsOn: setup) | |||
| File configDir = new File(node.homeDir, 'config') | |||
| copyConfig.into(configDir) // copy must always have a general dest dir, even though we don't use it | |||
| for (Map.Entry<String,Object> extraConfigFile : node.config.extraConfigFiles.entrySet()) { | |||
There was a problem hiding this comment.
In the other groovy files for buildSrc, I've been trying to use as little groovy stuff as possible. Can we keep the simple for loop here? It seems the changes to this file are just cosmetic.
There was a problem hiding this comment.
It isn't cosmetic! Without it every config file gets the contents of the last config file. I have no clue why. I did a ton of playing with it and eventually tried out this and it works. I have no idea how this is better than the old way but it works.
I wouldn't have made the change if it were simply cosmetic.
There was a problem hiding this comment.
I did mean to leave a note about this but I got distracted.
There was a problem hiding this comment.
I'll rebase this and remove the change.
|
@rjernst I pushed an update. I'd love to work through the ClusterFormationTask thing in another way if you've got any ideas. That took most of the time I spent on this, sadly. |
nik9000
force-pushed
the
doc_tests_for_js
branch
2 times, most recently
from
97267d6 to
546352b
Compare
|
@rjernst could you have another look? I haven't removed the groovy-ism that was required for an unknown reason. |
|
@rjernst could you have a look at this at some point? I'd love to get these under the test umbrella one day. |
1 similar comment
|
@rjernst could you have a look at this at some point? I'd love to get these under the test umbrella one day. |
|
I looked again, and fixed the gradle integ test issue with extra config files. My concern is still about requiring every doc test to add a "wait for the cluster to be ready". This should just be part of the setup. |
|
nik9000
force-pushed
the
doc_tests_for_js
branch
from
546352b to
8d2c27c
Compare
I did not know that. LGTM then for now. Looking forward to #18759 |
amen |
We weren't doing it before because we weren't starting the plugins. Now we are. The hardest part of this was handling the files the tests expect to be on the filesystem. extraConfigFiles was broken.
nik9000
force-pushed
the
doc_tests_for_js
branch
from
a4c4cf0 to
9c85569
Compare
We weren't doing it before because we weren't starting the plugins.
Now we are.
The hardest part of this was handling the files the tests expect
to be on the filesystem. extraConfigFiles was broken.