Skip to content

Conversation

@severussundar
Copy link
Contributor

@severussundar severussundar commented Nov 17, 2022

Why make this change?

What is this change?

  • The default config dab-config.json file is removed
  • In the build pipeline, the config files are not generated along with the build
  • Unit tests that had the file IO, deserialization and connecting to an actual database in its scope are converted to integrated tests.

To generate the final NuGet and Zips without the config files, we will not be generating the config files in the build pipeline going forward. However, there are some unit tests that performs validations on scenarios where reading the config file is mandatory. For example: Validating that once engine is started with a config file, then it would not possible to provide another using /configuration endpoint. Since, these tests need to read the config file and the config files are generated only in the integration testing pipelines, these tests are run as integration tests. Tagging them with the test category helps achieve this.

  • Few config json strings are introduced in the helper class TestHelper for unit tests that require the config json
  • EndToEndTests.TestEngineStartUpWithVerboseAndLogLevelOptions :
     1. Testing whether the engine starts correctly when different --LogLevels options are specified. 
     2. It was also testing whether the engine stops when an invalid config file is supplied.
     
     It was not testing whether the messages are getting logged at the specified log levels. When the engine fails to start, the CLI writes an error message using Console.Error.WriteLine() irrespective of the failure being due to invalid config/invalid option for log level. The test was asserting for that message. 
     
     As part of this change,
     
     1. The test summary is updated to reflect that only engine start-up with different available --LogLevel options is validated.
     2. Checking engine failure with an invalid config is removed as there is another test (`TestExitOfRuntimeEngineWithInvalidConfig`) that performs the same validation. 
     
     We would need another test to validate that the messages are getting logged at the appropriate level when --LogLevel option is used. 

How was this tested?

  • Integration Tests
  • Unit Tests
  • Manually verified the binaries that no config files are present

Sample Request(s)

  • N/A

@severussundar severussundar changed the title Dev/shyamsundarj/create nuget without configs Create binaries without config files Nov 17, 2022
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Left few questions, otherwise LGTM.

Copy link
Contributor

@jarupatj jarupatj left a comment

Choose a reason for hiding this comment

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

Need to have better PR descriptions. Some comments that you replied to me should have been in the PR description since the beginning.

PR description should explain "why" you are doing what you are doing. Right now, it just describes "what".

@Aniruddh25 Aniruddh25 merged commit 360e016 into main Nov 20, 2022
@Aniruddh25 Aniruddh25 deleted the dev/shyamsundarj/create-nuget-without-configs branch November 20, 2022 00:08
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.

Create the nuget packages and zip files without config files

4 participants