Skip to content

Commit 0376a01

Browse files
johljujohlju
authored
Refine and enhance clarity in Copilot instructions (#2093)
1 parent f9eda7b commit 0376a01

File tree

2 files changed

+287
-55
lines changed

2 files changed

+287
-55
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 284 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,212 @@
11
# Specific instructions for the PowerShell module project SqlServerDsc
22

3-
Assume that the word "command" references to a public command, and the word
4-
"function" references to a private function.
3+
Assume that the word "command" references to a public command, the word
4+
"function" references to a private function, and the word "resource"
5+
references a Desired State Configuration (DSC) class-based resource.
6+
7+
## Public commands
58

69
PowerShell commands that should be public should always have its separate
7-
script file and the the command name as the file name with the .ps1 extension,
10+
script file and the command name as the file name with the .ps1 extension,
811
these files shall always be placed in the folder source/Public.
912

1013
Public commands may use private functions to move out logic that can be
1114
reused by other public commands, so move out any logic that can be deemed
12-
reusable. Private functions should always have its separate script file and
13-
the the function name as the file name with the .ps1 extension, these files
14-
shall always be placed in the folder source/Private.
15-
16-
Comment-based help should be added to each public command and private functions.
17-
The comment-based help should always be before the function-statement. Each
18-
comment-based help keyword should be indented with 4 spaces and each keywords
19-
text should be indented 8 spaces. The text for keyword .DESCRIPTION should
20-
be descriptive and must have a length greater than 40 characters. A comment-based
21-
help must have at least one example, but preferably more examples to showcase
22-
all possible parameter sets and different parameter combinations.
15+
reusable.
16+
17+
## Private functions
18+
19+
Private functions (also known as helper functions) should always have its
20+
separate script file and the function name as the file name with the .ps1
21+
extension. These files shall always be placed in the folder source/Private.
22+
This also applies to functions that are only used within a single public
23+
command.
24+
25+
## Desired State Configuration (DSC) class-based resource
26+
27+
Desired State Configuration (DSC) class-based resource should always have
28+
its separate script file and the resource class name as the file name with
29+
the .ps1 extension, these files shall always be placed in the folder
30+
source/Classes.
31+
32+
### Parent classes
33+
34+
#### ResourceBase
35+
36+
A derived class should only inherit from the parent class `ResourceBase`
37+
when it can't inherit from `SqlResourceBase` or when the class-based resource
38+
is not related to SQL Server Database Engine.
39+
40+
The parent class `ResourceBase` will set up `$this.localizedData` and provide
41+
logic to compare the desired state against the current state. To get the
42+
current state it will call the overridable method `GetCurrentState`. If not
43+
in desired state it will call the overridable method `Modify`. It will also
44+
call the overridable methods `AssertProperties` and `NormalizeProperties` to
45+
validate and normalize the provided values of the desired state.
46+
47+
#### SqlResourceBase
48+
49+
A derived class should only inherit from the parent class `SqlResourceBase`
50+
if the class-based resource need to connect to a SQL Server Database Engine.
51+
52+
The parent class `SqlResourceBase` provides the DSC properties `InstanceName`,
53+
`ServerName`, `Credential` and `Reasons` and the method `GetServerObject`
54+
which is used to connect to a SQL Server Database Engine instance.
55+
56+
### Derived class
57+
58+
The derived class should use the decoration `[DscResource(RunAsCredential = 'Optional')]`.
59+
60+
The derived class should always inherit from a parent class.
61+
62+
The derived class should override the methods `Get`, `Test`, `Set`, `GetCurrentState`,
63+
`Modify`, `AssertProperties`, and `NormalizeProperties` using this pattern
64+
(and replace MyResourceName with actual resource name):
65+
66+
```powershell
67+
[MyResourceName] Get()
68+
{
69+
# Call the base method to return the properties.
70+
return ([ResourceBase] $this).Get()
71+
}
72+
73+
[System.Boolean] Test()
74+
{
75+
# Call the base method to test all of the properties that should be enforced.
76+
return ([ResourceBase] $this).Test()
77+
}
78+
79+
[void] Set()
80+
{
81+
# Call the base method to enforce the properties.
82+
([ResourceBase] $this).Set()
83+
}
84+
85+
<#
86+
Base method Get() call this method to get the current state as a hashtable.
87+
The parameter properties will contain the key properties.
88+
#>
89+
hidden [System.Collections.Hashtable] GetCurrentState([System.Collections.Hashtable] $properties)
90+
{
91+
# Add code to return the current state as an hashtable.
92+
}
93+
94+
<#
95+
Base method Set() call this method with the properties that are not in
96+
desired state and should be enforced. It is not called if all properties
97+
are in desired state. The variable $properties contains only the properties
98+
that are not in desired state.
99+
#>
100+
hidden [void] Modify([System.Collections.Hashtable] $properties)
101+
{
102+
# Add code to set the desired state based on the properties that are not in desired state.
103+
}
104+
105+
<#
106+
Base method Assert() call this method with the properties that was assigned
107+
a value.
108+
#>
109+
hidden [void] AssertProperties([System.Collections.Hashtable] $properties)
110+
{
111+
# Add code to validate class properties that the user passed values to.
112+
}
113+
114+
<#
115+
Base method Normalize() call this method with the properties that was assigned
116+
a value.
117+
#>
118+
hidden [void] NormalizeProperties([System.Collections.Hashtable] $properties)
119+
{
120+
# Add code to normalize class properties that the user passed values to.
121+
}
122+
```
123+
124+
## Comment-based help
125+
126+
Comment-based help should always be before the function-statement for each
127+
public command and private function, and before the class-statement for each
128+
class-based resource. Comment-based help should always be in the format of
129+
a comment block and at least use the keywords: .SYNOPSIS, .DESCRIPTION,
130+
.PARAMETER, .EXAMPLE, and .NOTES.
131+
132+
Each comment-based help keyword should be indented with 4 spaces and each
133+
keyword's text should be indented 8 spaces.
134+
135+
The text for keyword .DESCRIPTION should be descriptive and must have a
136+
length greater than 40 characters. The .SYNOPSIS keyword text should be
137+
a short description of the public command, private function, or class-based
138+
resource.
139+
140+
A comment-based help must have at least one example, but preferably more
141+
examples to showcase all possible parameter sets and different parameter
142+
combinations.
143+
144+
## Localization
23145

24146
All message strings for Write-Debug, Write-Verbose, Write-Error, Write-Warning
25147
and other error messages in public commands and private functions should be
26-
localized using localized string keys. You should always add all localized
27-
strings for public commands and private functions in the source/en-US/SqlServerDsc.strings.psd1
28-
file, re-use the same pattern for new string keys. Localized string key names
29-
should always be prefixed with the function name but use underscore as word
30-
separator. Always assume that all localized string keys have already been
31-
assigned to the variable $script:localizedData.
148+
localized using localized string keys.
32149

33-
All tests should use the Pester framework and use Pester v5.0 syntax.
150+
For public commands and private functions you should always add all localized
151+
strings for in the source/en-US/SqlServerDsc.strings.psd1 file, re-use the
152+
same pattern for new string keys. Localized string key names should always
153+
be prefixed with the function name but use underscore as word separator.
154+
Always assume that all localized string keys have already been assigned to
155+
the variable $script:localizedData.
34156

35-
Never test, mock or use `Should -Invoke` for `Write-Verbose` and `Write-Debug`
36-
regardless of other instructions.
157+
For class-based resource you should always add a localized strings in a
158+
separate file the folder source\en-US. The strings file for a class-based
159+
resource should be named to exactly match the resource class name with the
160+
suffix `.strings.psd1`.
161+
Localized string key names should use underscore as word separator if key
162+
name has more than one word. Always assume that all localized string keys
163+
for a class-based resource already have been assigned to the variable
164+
`$this.localizedData` by the parent class.
37165

38-
Test code should never be added outside of the `Describe` block.
166+
## Tests
167+
168+
All tests should use the Pester framework and use Pester v5.0 syntax.
169+
Parameter validation should never be tested.
39170

40-
Unit tests should be added for all public commands and private functions.
41-
The unit tests for public command should be placed in the folder tests/Unit/Public
42-
and the unit tests for private functions should be placed in the folder
43-
tests/Unit/Private. The unit tests should be named after the public command
44-
or private function they are testing, but should have the suffix .Tests.ps1.
45-
The unit tests should be written to cover all possible scenarios and code paths,
46-
ensuring that both edge cases and common use cases are tested.
171+
Test code should never be added outside of the `Describe` block.
47172

48173
There should only be one Pester `Describe` block per test file, and the name of
49-
the `Describe` block should be the same as the name of the public command or
50-
private function being tested. Each scenario or code path being tested should
51-
have its own Pester `Context` block that starts with the phrase 'When'. Use
52-
nested `Context` blocks to split up test cases and improve tests readability.
53-
Pester `It` block descriptions should start with the phrase 'Should'. `It`
54-
blocks must always call the command or function being tested and result and
55-
outcomes should be kept in the same `It` block. `BeforeAll` and `BeforeEach`
56-
blocks should never call the command or function being tested.
174+
the `Describe` block should be the same as the name of the public command,
175+
private function, or class-based resource being tested. Each scenario or
176+
code path being tested should have its own Pester `Context` block that starts
177+
with the phrase 'When'. Use nested `Context` blocks to split up test cases
178+
and improve tests readability. Pester `It` block descriptions should start
179+
with the phrase 'Should'. `It` blocks must always call the command or function
180+
being tested and result and outcomes should be kept in the same `It` block.
181+
`BeforeAll` and `BeforeEach` blocks should never call the command or function
182+
being tested.
57183

58184
The `BeforeAll`, `BeforeEach`, `AfterAll` and `AfterEach` blocks should be
59185
used inside the `Context` block as near as possible to the `It` block that
60-
will use the mocked test setup and teardown. The `BeforeAll` block should
61-
be used to set up any necessary test data or mocking, and the `AfterAll`
62-
block can be used to clean up any test data. The `BeforeEach` and `AfterEach`
186+
will use the test data, test setup and teardown. The `AfterAll` block can
187+
be used to clean up any test data. The `BeforeEach` and `AfterEach`
63188
blocks should be used sparingly. It is okay to duplicated code in `BeforeAll`
64-
and `BeforeEach` blocks inside different `Context` blocks to help with
65-
readability and understanding of the test cases, to keep the test setup
66-
and teardown as close to the test case as possible.
189+
and `BeforeEach` blocks that are used inside different `Context` blocks.
190+
The duplication helps with readability and understanding of the test cases,
191+
and to be able to keep the test setup and teardown as close to the test
192+
case (`It`-block) as possible.
193+
194+
### Unit tests
195+
196+
Never test, mock or use `Should -Invoke` for `Write-Verbose` and `Write-Debug`
197+
regardless of other instructions.
198+
199+
Unit tests should be added for all public commands, private functions and
200+
class-based resources. The unit tests for class-based resources should be
201+
placed in the folder tests/Unit/Classes. The unit tests for public command
202+
should be placed in the folder tests/Unit/Public and the unit tests for
203+
private functions should be placed in the folder tests/Unit/Private. The
204+
unit tests should be named after the public command or private function
205+
they are testing, but should have the suffix .Tests.ps1. The unit tests
206+
should be written to cover all possible scenarios and code paths, ensuring
207+
that both edge cases and common use cases are tested.
208+
209+
The `BeforeAll` block should be used to set up any necessary test data or mocking
67210

68211
Use localized strings in the tests only when necessary. You can assign the
69212
localized string to a mock variable by and get the localized string key
@@ -132,20 +275,36 @@ AfterAll {
132275
}
133276
```
134277

278+
### Integration tests
279+
280+
Integration tests that depend on an SQL Server Database Engine instance
281+
will run in environments in CI/CD pipelines where an instance DSCSQLTEST
282+
is already installed. Each environment will have SQL Server 2016, 2017,
283+
2019, or 2022.
284+
285+
Integration tests that depend on an SQL Server Reporting services instance
286+
will run in environments in CI/CD pipelines where an instance SSRS is already
287+
installed. Each environment will have SQL Server Reporting services 2016,
288+
2017, 2019, or 2022.
289+
290+
Integration tests that depend on a Power BI Report Server instance
291+
will run in one environment in CI/CD pipeline where an instance PBIRS is
292+
already installed. The environment will always have the latest Power BI
293+
Report Server version.
294+
135295
Integration tests should be added for all public commands. Integration must
136-
never mock any command but run the command in a real environment. The integration
137-
tests should be placed in the folder tests/Integration/Commands and the
138-
integration tests should be named after the public command they are testing,
139-
but should have the suffix .Integration.Tests.ps1. The integration tests should be
140-
written to cover all possible scenarios and code paths, ensuring that both
296+
never mock any command but run the command in a real environment. All integration
297+
tests should be placed in the root of the folder "tests/Integration/Commands"
298+
and the integration tests should be named after the public command they are testing,
299+
but should have the suffix .Integration.Tests.ps1. The integration tests should
300+
be written to cover all possible scenarios and code paths, ensuring that both
141301
edge cases and common use cases are tested. The integration tests should
142302
also be written to test the command in a real environment, using real
143303
resources and dependencies.
144304

145-
The module being tested should not be imported in the integration tests.
146-
All integration tests should should use this code block prior to the `Describe`
147-
block which will set up the test environment and will make sure the correct
148-
module is available for testing:
305+
All integration tests must use the below code block prior to the first
306+
`Describe`-block. The following code will set up the integration test
307+
environment and it will make sure the module being tested is available
149308

150309
```powershell
151310
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')]
@@ -172,4 +331,75 @@ BeforeDiscovery {
172331
throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks build" first.'
173332
}
174333
}
334+
335+
BeforeAll {
336+
$script:dscModuleName = 'SqlServerDsc'
337+
338+
Import-Module -Name $script:dscModuleName
339+
}
175340
```
341+
342+
The module DscResource.Test is used by the pipeline and its commands
343+
are normally not used when testing public functions, private functions or
344+
class-based resources.
345+
346+
## Style guidelines
347+
348+
This project use the style guidelines from the DSC Community: https://dsccommunity.org/styleguidelines
349+
350+
### Markdown files
351+
352+
- Line length should be wrapped after a word when a line exceeds 80 characters
353+
in length.
354+
- Use 2 spaces for indentation in Markdown files.
355+
356+
### PowerShell files
357+
358+
- All files should use UTF8 without BOM.
359+
360+
### PowerShell code
361+
362+
- Try to limit lines to 120 characters
363+
- Use 4 spaces for indentation, never use tabs.
364+
- Use '#' for single line comments
365+
- Use comment-blocks for multiline comments with the format `<# ... #>`
366+
where the multiline text is indented 4 spaces.
367+
- Use descriptive, clear, and full names for all variables, parameters, and
368+
function names. All names must be more than 2 characters. No abbreviations
369+
should be used.
370+
- Use camelCase for local variable names
371+
- Use PascalCase for function names and parameters in public commands, private
372+
functions and class-based resources.
373+
- All public command and private function names must follow the standard
374+
PowerShell Verb-Noun format.
375+
- All public command and private function names must use PowerShell approved
376+
verbs.
377+
- All public commands and private functions should always be advanced functions
378+
and have `[CmdLetBinding()]` attribute.
379+
- Public commands and private functions with no parameters should still have
380+
an empty parameter block `param ()`.
381+
- Every parameter in public commands and private functions should include the
382+
`[Parameter()]` attribute.
383+
- A mandatory parameter in public commands and private functions should
384+
contain the decoration `[Parameter(Mandatory = $true)]`, and non-mandatory
385+
parameters should not contain the Mandatory decoration.
386+
- Parameters attributes, datatype and its name should be on separate lines.
387+
- Parameters must be separated by a single, blank line.
388+
- Use `Write-Verbose` to output verbose output for actions an public command
389+
or private function does.
390+
- Use `Write-Debug` for debug output for processes within the script for
391+
user to see the decisions a command och private function does.
392+
- Use `Write-Error` for error messages.
393+
- Use `Write-Warning` for warning messages.
394+
- Use `Write-Information` for informational messages.
395+
- Never use `Write-Host`.
396+
- Never use backtick as line continuation in code.
397+
- Use splatting for commands to reduce line length.
398+
- PowerShell reserved Keywords should be in all lower case.
399+
- Single quotes should always be used to delimit string literals wherever
400+
possible. Double quoted string literals may only be used when string
401+
literals contain ($) expressions that need to be evaluated.
402+
- Hashtables properties should always be in PascalCase and be on a separate
403+
line.
404+
- When comparing a value to `$null`, `$null` should be on the left side of
405+
the comparison.

‎CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6868
the module manifest.
6969
- Now integration tests will fail on an exception when the command `Test-DscConfiguration`
7070
is run.
71-
- Added Test-SqlDscIsRole to be used like Test-SqlDscIsLogin but tests for a server role as principal.
71+
- Added Test-SqlDscIsRole to be used like Test-SqlDscIsLogin but tests
72+
for a server role as principal.
73+
- Refine and enhance clarity in Copilot instructions.
7274
- SqlSetup
7375
- Fixed issue with AddNode where cluster IP information was not being passed to
7476
setup.exe ([issue #1171](https://github.com/dsccommunity/SqlServerDsc/issues/1171)).

0 commit comments

Comments
 (0)