715_Create_tests_roomBooking #1151

Merged
a22erigr merged 30 commits from 715_Create_tests_roomBooking into both_teams_week_8 2026-05-28 07:39:35 +00:00
Collaborator

Have started on the base for the integration tests for roombooking.
this uses the structure from @a24hirsa tests for energyservice.

did not have the time to make proper test so this is just the base structure for furthure tests.

should work the same as #1113 with the exeption of that it has been modified to work with roombooking

Have started on the base for the integration tests for roombooking. this uses the structure from @a24hirsa tests for energyservice. did not have the time to make proper test so this is just the base structure for furthure tests. should work the same as #1113 with the exeption of that it has been modified to work with roombooking
Author
Collaborator

as this is only a start the issue should still be open

as this is only a start the issue should still be open
Collaborator

Please remember that all the comments will be visible in the git history. Have this in mind when writing both commit messages and comments.

Please remember that all the comments will be visible in the git history. Have this in mind when writing both commit messages and comments.
a24timsv requested changes 2026-05-26 08:54:17 +00:00
Dismissed
a24timsv left a comment
Collaborator

image
The test executes and I can see the test container in Docker being alive, then dying and the test concluding. I am definitely no expert in this field either, but if this follows the energy service as a template then I'm pretty sure it works as you guys intend.

There is some other issues with the code however, mostly related to the comments. First of all, as Eric mentioned, git history is saved so we should try to avoid leaving... spicy git messages and code comments, as they are saved as history. Try to keep this in mind for future commits, as this is the same on GitHub.

Furthermore:

  • The comments are written by one line (//) comments instead of /* */, not following the coding conventions for longer comments.
  • There are also some spelling mistakes in the comments.
  • Line 206 is written in Swedish.
  • The comments you have written has a tone of uncertainty. We shouldn't push code we don't know works or we are uncertain that it work.

Changes needed

  • Rewrite and fix the comments according to our coding conventions.
![image](/attachments/c4009bb4-9b98-40dd-9771-354f5b8b0bf6) The test executes and I can see the test container in Docker being alive, then dying and the test concluding. I am definitely no expert in this field either, but if this follows the energy service as a template then I'm pretty sure it works as you guys intend. There is some other issues with the code however, mostly related to the comments. First of all, as Eric mentioned, git history is saved so we should try to avoid leaving... spicy git messages and code comments, as they are saved as history. Try to keep this in mind for future commits, as this is the same on GitHub. Furthermore: - The comments are written by one line (//) comments instead of /* */, not following the coding conventions for longer comments. - There are also some spelling mistakes in the comments. - Line 206 is written in Swedish. - The comments you have written has a tone of uncertainty. We shouldn't push code we don't know works or we are uncertain that it work. ## Changes needed - Rewrite and fix the comments according to our coding conventions.
a22erigr changed target branch from team_1_week_7 to both_teams_week_8 2026-05-27 08:57:49 +00:00
Author
Collaborator

rebased the branch to both_teams_week_8

rebased the branch to both_teams_week_8
a24timsv requested changes 2026-05-27 12:04:31 +00:00
Dismissed
a24timsv left a comment
Collaborator

Comments are still misspelled and hard to read. See example provided below.

I checked with @a24hirsa to guide through this entire thing, and the test works but I needed to delete all my existing containers. This should not be needed and the fixes proposed should cover this.

Comments are still misspelled and hard to read. See example provided below. I checked with @a24hirsa to guide through this entire thing, and the test works but I needed to delete all my existing containers. This should not be needed and the fixes proposed should cover this.
@ -0,0 +51,4 @@
/*what services/images that need to be build vary between different services,
once a image is made you must CreateAsync() before usign it in a ContainerBuilder */
_postgres = new PostgreSqlBuilder("postgres:18.3-alpine3.23")
.WithImage("postgres:15-alpine")
Collaborator

WithImage can be deleted, as it is not necessary.

WithImage can be deleted, as it is not necessary.
a24antej marked this conversation as resolved
@ -0,0 +56,4 @@
.WithEnvironment("POSTGRES_PASSWORD", dbPassword)
.WithEnvironment("POSTGRES_DB", "sensors")
.WithEnvironment("POSTGRES_HOST_AUTH_METHOD", "trust")
.WithHostname("roombookingservice")
Collaborator

The test container should not, and does not need a WithHostname or WithName. It just adds conflicts with existing containers.

Applies to line: 59, 60, 75 and 76

The test container should not, and does not need a WithHostname or WithName. It just adds conflicts with existing containers. Applies to line: 59, 60, 75 and 76
a24antej marked this conversation as resolved
@ -0,0 +91,4 @@
builder.ConfigureServices(services =>
{
/*currently services use diffrent DataSource names to connect to msqtt and similar thigns
Collaborator
/* Currently services use different DataSource names to connect to MQTT and similar things
 * These don't carry over into the testcontainers so we need to basically override/remove and replace them
 * so that the test webserver can be used
 */

Using this comment block as an example for others. Some things are still misspelled, abbreviations are small case, stuff like that.
While this is not in our code conventions, /* */ blocks structured like the example above makes them easier to read and easier to delete. As far as I know, this is commonly used in the industry.

I recommend going over all comments and applying the same method of the example above.

``` /* Currently services use different DataSource names to connect to MQTT and similar things * These don't carry over into the testcontainers so we need to basically override/remove and replace them * so that the test webserver can be used */ ``` Using this comment block as an example for others. Some things are still misspelled, abbreviations are small case, stuff like that. While this is not in our code conventions, /* */ blocks structured like the example above makes them easier to read and easier to delete. As far as I know, this is commonly used in the industry. I recommend going over all comments and applying the same method of the example above.
Author
Collaborator

fixed as much as i can find, if there are still some misspellings please tell where(if you whant them to be fixed)
should else be acceptable enought as it is now.

fixed as much as i can find, if there are still some misspellings please tell where(if you whant them to be fixed) should else be acceptable enought as it is now.
@ -0,0 +166,4 @@
{
/*Test if http requests work by just checking if health returns
prefixes can be different for services.*/
var response = await _httpClient.GetAsync("/roombooking/healthcheck");
Collaborator

Wrong endpoint. Should be

var response = await _httpClient.GetAsync("/roombooking/health");
Wrong endpoint. Should be ```csharp var response = await _httpClient.GetAsync("/roombooking/health"); ```
a24antej marked this conversation as resolved
removed some unecesary code on container builds,
fixed comments to more suite the coding standard, removed some unececaryh comments.
may still be some misspelings cause im bad att finding that stuff
a24timsv requested changes 2026-05-27 14:31:09 +00:00
Dismissed
a24timsv left a comment
Collaborator

Highlighted the spelling mistakes I could find.

Also, I highly recommend reading the C# coding conventions about comments. It talks about capitalization, spacing and ending comments with a period. This applies to all comments.

Highlighted the spelling mistakes I could find. Also, I highly recommend reading the [C# coding conventions](https://git.webug.se/Andras/BoundlessFlowCampus2K/wiki/C%23-Coding-Convention#comments) about comments. It talks about capitalization, spacing and ending comments with a period. This applies to all comments.
@ -0,0 +104,4 @@
services.AddSingleton(testDataSource);
/* We create a postgresql container, a place for databases exist but it seems to still need to be populated.
* This helps create a real database and not a mock database so all tests ran on it would work on the actual database druing run time.
Collaborator

Spelling, druing

Spelling, _druing_
a24antej marked this conversation as resolved
@ -0,0 +177,4 @@
[Fact]
public async Task Get_bookingData_Returns_Success_And_Data()
{
/* http request after populating with the table. Again the database table is real, just empty so we create it durign cotnainer building
Collaborator

Spelling, durign cotnainer

Spelling, _durign cotnainer_
a24antej marked this conversation as resolved
@ -0,0 +179,4 @@
{
/* http request after populating with the table. Again the database table is real, just empty so we create it durign cotnainer building
* and in tests we can populate the table.
* There should be some way to populate them as normal usign your generators but testcontainers are fast,
Collaborator

usign as in unsigned or misspelling of using?

usign as in unsigned or misspelling of _using_?
a24antej marked this conversation as resolved
@ -0,0 +181,4 @@
* and in tests we can populate the table.
* There should be some way to populate them as normal usign your generators but testcontainers are fast,
* once built they run the tests extremly quickly and then kill the contianers
* so you propably don't even have time to passivly populate the containers.
Collaborator

Spelling, passivly

Spelling, _passivly_
a24antej marked this conversation as resolved
a24timsv approved these changes 2026-05-28 07:37:44 +00:00
a24timsv left a comment
Collaborator

Looks much better. Is ready to be merged

Looks much better. Is ready to be merged
a22erigr merged commit 718204fe3a into both_teams_week_8 2026-05-28 07:39:35 +00:00
a22erigr deleted branch 715_Create_tests_roomBooking 2026-05-28 07:39:35 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Andras/BoundlessFlowCampus2K!1151
No description provided.