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

Tests use typescript #534

Merged
merged 13 commits into from Feb 19, 2021
Merged

Tests use typescript #534

merged 13 commits into from Feb 19, 2021

Conversation

gugu
Copy link
Contributor

@gugu gugu commented Feb 10, 2021

Tests switched to ts.

  1. PR includes async cache changes for now
  2. I did not add types for the request module - we need to remove this dinosaur instead of adding types to it

@cjbarth
Copy link
Collaborator

cjbarth commented Feb 10, 2021

I'll review this when the cache code is merged and this is rebased on top of it.

@cjbarth cjbarth marked this pull request as draft February 10, 2021 15:17
@gugu
Copy link
Contributor Author

gugu commented Feb 10, 2021

@cjbarth done

Copy link
Collaborator

@cjbarth cjbarth left a comment

Choose a reason for hiding this comment

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

I found some simple things that need adjustment. I've also prepared some changes and pushed them to clean up the testing framework.

test/multiSamlStrategy.ts Outdated Show resolved Hide resolved
test/multiSamlStrategy.ts Outdated Show resolved Hide resolved
test/multiSamlStrategy.ts Outdated Show resolved Hide resolved
test/samlTests.ts Outdated Show resolved Hide resolved
test/test-signatures.ts Outdated Show resolved Hide resolved
test/tests.ts Outdated Show resolved Hide resolved
test/tests.ts Outdated Show resolved Hide resolved
test/tests.ts Outdated Show resolved Hide resolved
test/tests.ts Outdated Show resolved Hide resolved
test/tests.ts Outdated Show resolved Hide resolved
@cjbarth
Copy link
Collaborator

cjbarth commented Feb 15, 2021

I've merged in the head of master, but didn't get a chance to clean everything up.

@gugu gugu marked this pull request as ready for review February 18, 2021 15:45
There are still a few failing test, I think, in part, to the way we are no longer `require`ing `json` files.
@cjbarth
Copy link
Collaborator

cjbarth commented Feb 19, 2021

There are some tests that fail intermittently, I think because we have a test-interdependence. So, when Mocha runs the tests in random order, some tests occasionally fail. Other than that, I think this code is all better. I would love someone to take a look at the intermittent failures.

@gugu
Copy link
Contributor Author

gugu commented Feb 19, 2021

@cjbarth done. PassReqToCallback test modified shared config variable

Copy link
Collaborator

@cjbarth cjbarth left a comment

Choose a reason for hiding this comment

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

This all looks good. I think we should probably make a follow-on PR that removes all the deprecated code and converts tests that use deprecated code to use the newer code.

If you feel it looks good, either one of us can do a merge commit (though I would clean up the comment because it is a little crazy now :) ).

@gugu gugu merged commit f1a436f into master Feb 19, 2021
@gugu gugu deleted the tests-typescript branch February 19, 2021 17:40
@cjbarth cjbarth mentioned this pull request May 10, 2021
@cjbarth cjbarth added the chore Refactoring, etc. to keep code-rot in check. label May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactoring, etc. to keep code-rot in check.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants