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

Add testing templates #508

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Add testing templates #508

merged 6 commits into from
Feb 6, 2024

Conversation

phracek
Copy link
Member

@phracek phracek commented Nov 29, 2023

Add testing templates.

Templates were not tested. This pull request allows it.

@phracek phracek marked this pull request as draft November 29, 2023 11:50
@phracek phracek force-pushed the add_testing_templates branch from b846cce to 9e66fa4 Compare December 4, 2023 09:39
@phracek phracek marked this pull request as ready for review December 4, 2023 09:39
@phracek
Copy link
Member Author

phracek commented Dec 4, 2023

[test-openshift]

Templates were not tested. This pull request allows it.

Signed-off-by: Petr "Stone" Hracek <[email protected]>
@phracek phracek force-pushed the add_testing_templates branch from 10b6838 to 9f18df9 Compare January 19, 2024 11:41
@phracek
Copy link
Member Author

phracek commented Jan 19, 2024

Templates added. Tests are executed on rails.json template with no database.

[test-openshift]

@phracek
Copy link
Member Author

phracek commented Jan 19, 2024

Tested on rails-ex PR sclorg/rails-ex#166

@phracek phracek force-pushed the add_testing_templates branch from 9f18df9 to 1bf331e Compare January 19, 2024 13:09
@phracek
Copy link
Member Author

phracek commented Jan 19, 2024

[test-all]

Signed-off-by: Petr "Stone" Hracek <[email protected]>
@phracek
Copy link
Member Author

phracek commented Jan 22, 2024

[test-openshift]

@phracek
Copy link
Member Author

phracek commented Jan 22, 2024

[test]

@jackorp
Copy link
Contributor

jackorp commented Jan 22, 2024

@phracek Hmm, looking at this, we are now testing the app with no database?

However it seems you added some changes that import the postgresql image now, so it should work now, is that correct?

So are we expecting all 3 currently present templates (rails.json, rails-postgresql.json, rails-postgresql-persistent.json) to show up in the tests and be tested? I can only see rails.json AFAICT.

What do we expect to be tested then should be answered and I think all else is good to be merged, since the tests are green and the logs look OK from a short glance.

@phracek
Copy link
Member Author

phracek commented Jan 22, 2024

@phracek Hmm, looking at this, we are now testing the app with no database?

However it seems you added some changes that import the postgresql image now, so it should work now, is that correct?

So are we expecting all 3 currently present templates (rails.json, rails-postgresql.json, rails-postgresql-persistent.json) to show up in the tests and be tested? I can only see rails.json AFAICT.

What do we expect to be tested then should be answered and I think all else is good to be merged, since the tests are green and the logs look OK from a short glance.

@jackorp Thanks for this question. As you can see, the templates were not tested at all and this pull request only tests rails.json. In the future as soon as rails-postgresql-persistent.json works, I would like to add. I have to find out a way where to add a variable RAILS_LOG_TO_STDOUT="something".

@jackorp
Copy link
Contributor

jackorp commented Jan 22, 2024

I have to find out a way where to add a variable RAILS_LOG_TO_STDOUT="something".

diff --git a/examples/rails-postgresql-persistent.json b/examples/rails-postgresql-persistent.json
index 74a5d26..20547ee 100644
--- a/examples/rails-postgresql-persistent.json
+++ b/examples/rails-postgresql-persistent.json
@@ -279,6 +279,10 @@
                   {
                     "name": "RAILS_ENV",
                     "value": "${RAILS_ENV}"
+                  },
+                  {
+                    "name": "RAILS_LOG_TO_STDOUT",
+                    "value": "something"
                   }
                 ],
                 "resources": {
diff --git a/examples/rails-postgresql.json b/examples/rails-postgresql.json
index b482260..2f280a7 100644
--- a/examples/rails-postgresql.json
+++ b/examples/rails-postgresql.json
@@ -279,6 +279,10 @@
                   {
                     "name": "RAILS_ENV",
                     "value": "${RAILS_ENV}"
+                  },
+                  {
+                    "name": "RAILS_LOG_TO_STDOUT",
+                    "value": "something"
                   }
                 ],
                 "resources": {
diff --git a/examples/rails.json b/examples/rails.json
index a29a439..e4383b6 100644
--- a/examples/rails.json
+++ b/examples/rails.json
@@ -182,6 +182,10 @@
                   {
                     "name": "RAILS_ENV",
                     "value": "${RAILS_ENV}"
+                  },
+                  {
+                    "name": "RAILS_LOG_TO_STDOUT",
+                    "value": "something"
                   }
                 ],
                 "resources": {

This should work.
Since those deployments define RAILS_ENV, it should be the correct container.

@phracek
Copy link
Member Author

phracek commented Feb 5, 2024

@jackorp It looks like everything is passing. We can merge it. WDYT?

@phracek
Copy link
Member Author

phracek commented Feb 5, 2024

Sorry, I have missed your comment #508 (comment). What about to create an issue to rails-postgresql-persistent.json and this merge it?

Copy link
Contributor

@jackorp jackorp left a comment

Choose a reason for hiding this comment

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

LGTM.

@jackorp
Copy link
Contributor

jackorp commented Feb 5, 2024

What about to create an issue to rails-postgresql-persistent.json and this merge it?

Yeah, sure, sounds good. Let's address it separately.

Btw AFAICT I don't have write access to the repo so I can't give you the correct approval (since it requires approval from a write capable member)

@phracek phracek merged commit 708dea5 into master Feb 6, 2024
15 of 16 checks passed
@phracek phracek deleted the add_testing_templates branch February 6, 2024 08:56
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.

2 participants