Skip to content

chore: Update test docs to postgresql; remove extraneous sqlite stuff#5400

Merged
rjsparks merged 2 commits into
ietf-tools:feat/postgresfrom
jimfenton:testdoc
Mar 29, 2023
Merged

chore: Update test docs to postgresql; remove extraneous sqlite stuff#5400
rjsparks merged 2 commits into
ietf-tools:feat/postgresfrom
jimfenton:testdoc

Conversation

@jimfenton

Copy link
Copy Markdown
Collaborator

No description provided.

Comment on lines -57 to -67
if [ ! -f "$WORKSPACEDIR/ietf/settings_local_sqlitetest.py" ]; then
echo "Setting up a default settings_local_sqlitetest.py ..."
cp $WORKSPACEDIR/docker/configs/settings_local_sqlitetest.py $WORKSPACEDIR/ietf/settings_local_sqlitetest.py
else
echo "Using existing ietf/settings_local_sqlitetest.py file"
if ! cmp -s $WORKSPACEDIR/docker/configs/settings_local_sqlitetest.py $WORKSPACEDIR/ietf/settings_local_sqlitetest.py; then
echo "NOTE: Differences detected compared to docker/configs/settings_local_sqlitetest.py!"
echo "We'll assume you made these deliberately."
fi
fi

@rjsparks rjsparks Mar 26, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@NGPixel I don't think any devs used settings_local_sqlitetest. Did any actions? We don't have a comparable settings_local_postgrestest, and I suspect we don't need one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But that's in the main branch, not feat/postgres. The corresponding lines are:

https://github.com/ietf-tools/datatracker/blob/feat/postgres/.github/workflows/build.yml#L156-L158

which use settings_postgrestest, not settings_local_sqlitetest

Comment thread ietf/.gitignore Outdated
/settings_local_vite.py
/settings_mysqldb.py
/settings_postgresqldb.py
/settings_postgrestest.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file is in git and does not need to be ignored. Please revert this change

@jimfenton jimfenton Mar 26, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reverted by 012de38 although I don't quite understand why to .gitignore settings_postgresqldb.py and not settings_postgrestest.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

settings_postgresqldb.py is source-controlled in docker/configs/, and is copied into ietf/ as the dev environment is built - it's runtime configuration for the actual app, not the tests. settings_postgrestest.py is source-controlled in ietf/.

That schism is an artifact of history, and we could normalize what we're doing, but that's a bigger task than this PR should take on.

@rjsparks
rjsparks requested a review from NGPixel March 26, 2023 04:55
@codecov

codecov Bot commented Mar 26, 2023

Copy link
Copy Markdown

Codecov Report

Merging #5400 (4041d83) into feat/postgres (736382a) will increase coverage by 0.00%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##           feat/postgres    #5400   +/-   ##
==============================================
  Coverage          88.80%   88.80%           
==============================================
  Files                285      285           
  Lines              39525    39529    +4     
==============================================
+ Hits               35100    35104    +4     
  Misses              4425     4425           

see 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jimfenton
jimfenton requested review from rjsparks and removed request for NGPixel March 26, 2023 11:09
@rjsparks

Copy link
Copy Markdown
Member

Jim - I think something went wrong with a rebase here - can could look and clean it back to the intended changes?

@jimfenton

Copy link
Copy Markdown
Collaborator Author

Yes, I did a sync of my fork to the main repository and it seems to have done bad things. I haven't run into that before, probably because I wasn't working on such an active project.

Let me see if I can fix it up; otherwise I'll just close this PR and create a new one.

@jimfenton

Copy link
Copy Markdown
Collaborator Author

@rjsparks With help from Justin Richer, I got this pull request cleaned up. Please take another look.

@rjsparks
rjsparks merged commit 0f1a6c9 into ietf-tools:feat/postgres Mar 29, 2023
@jimfenton
jimfenton deleted the testdoc branch March 30, 2023 08:16
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants