Skip to content

Conversation

@hezhangjian
Copy link
Member

Motivation

Make AuthenticationTokenTest to run on windows

Modifications

Replace the '' to '/'

Documentation

  • no-need-doc

little unit test change, doesn't need doc

@hezhangjian
Copy link
Member Author

/pulsarbot run-failure-checks

1 similar comment
@hezhangjian
Copy link
Member Author

/pulsarbot run-failure-checks

@hezhangjian
Copy link
Member Author

@codelipenghui @BewareMyPower @sijie PTAL, thanks

@BewareMyPower
Copy link
Contributor

Could you explain why it cannot run on Windows? I see the core change is changing

"file://" + tokenFile

to

"file:///" + tokenFile.toString().replace('\\', '/')

Could you give an example for what File.createTempFile could return on Windows?

@hezhangjian
Copy link
Member Author

hezhangjian commented Oct 12, 2021

Could you explain why it cannot run on Windows? I see the core change is changing

"file://" + tokenFile

to

"file:///" + tokenFile.toString().replace('\\', '/')

Could you give an example for what File.createTempFile could return on Windows?

Run in windows can't recognize format like file://C\xxx\yyy\zzz

It should be file:///C and the \ should be /

@BewareMyPower BewareMyPower added the doc-not-needed Your PR changes do not impact docs label Oct 12, 2021
@BewareMyPower BewareMyPower merged commit 5f00b87 into apache:master Oct 13, 2021
@hezhangjian hezhangjian deleted the auth-token-windows-ut branch October 13, 2021 01:57
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Oct 14, 2021
* up/master: (26 commits)
  [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128)
  [website][upgrade]feat: docs migration - Development (apache#12320)
  Update delete inactive topic configuration documentation (apache#12350)
  [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056)
  Added Debezium Source for MS SQL Server (apache#12256)
  Fix: flaky oracle tests (apache#12306)
  [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341)
  [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335)
  feat(cli): add restart command to pulsar-daemon (apache#12279)
  [client-tools] Remove redundant initial value (apache#12296)
  Make AuthenticationTokenTest to run on windows (apache#12329)
  [offload] fix FileSystemManagedLedgerOffloader can not cleanup outdated ledger data (apache#12309)
  [Doc]--Update contents for Pulsar adaptor for Apache Spark (apache#12338)
  [PIP 95][Issue 12040][broker] Improved multi-listener in standalone mode (apache#12066)
  [website][upgrade]feat: docs migration - Cookbooks (apache#12319)
  [testclient] Make --payload-file take effect in PerformanceClient (apache#12187)
  [website][upgrade]feat: docs migration - adaptor (apache#12318)
  [pulsar-client] Add partition-change api for producer/consumer interceptors (apache#12287)
  [Transaction]Fix lowWaterMark of TopicTransactionBuffer (apache#12312)
  [pulsar-admin] New option takes precedence over deprecated option (apache#12260)
  ...

# Conflicts:
#	site2/website-next/docusaurus.config.js
#	site2/website-next/versions.json
BewareMyPower pushed a commit that referenced this pull request Oct 19, 2021
### Motivation
Make unit test path compatible to windows, make these tests can run on windows

### Modifications

Same to #12329 , change `"file://" + tokenFile` to `"file:///" + tokenFile.toString().replace('\\', '/')`

Run in windows can't recognize format like `file://C\xxx\yyy\zzz`

It should be `file:///C` and the `\` should be `/`
codelipenghui pushed a commit that referenced this pull request Oct 21, 2021
### Motivation
Make AuthenticationTokenTest to run on windows

### Modifications
Replace the '\' to '/'

(cherry picked from commit 5f00b87)
codelipenghui pushed a commit that referenced this pull request Oct 21, 2021
### Motivation
Make unit test path compatible to windows, make these tests can run on windows

### Modifications

Same to #12329 , change `"file://" + tokenFile` to `"file:///" + tokenFile.toString().replace('\\', '/')`

Run in windows can't recognize format like `file://C\xxx\yyy\zzz`

It should be `file:///C` and the `\` should be `/`

(cherry picked from commit 4da4310)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Oct 21, 2021
@codelipenghui codelipenghui modified the milestones: 2.9.0, 2.10.0 Oct 21, 2021
codelipenghui pushed a commit that referenced this pull request Dec 20, 2021
### Motivation
Make unit test path compatible to windows, make these tests can run on windows

### Modifications

Same to #12329 , change `"file://" + tokenFile` to `"file:///" + tokenFile.toString().replace('\\', '/')`

Run in windows can't recognize format like `file://C\xxx\yyy\zzz`

It should be `file:///C` and the `\` should be `/`

(cherry picked from commit 4da4310)
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation
Make AuthenticationTokenTest to run on windows

### Modifications
Replace the '\' to '/'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/test cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants